Re: [PATCH net] sctp: call rcu_read_lock before checking for duplicate transport nodes
From: Neil Horman <nhorman@tuxdriver.com>
Date: 2017-02-28 14:24:28
Also in:
linux-sctp
On Tue, Feb 28, 2017 at 12:41:29PM +0800, Xin Long wrote:
quoted hunk ↗ jump to hunk
Commit cd2b70875058 ("sctp: check duplicate node before inserting a new transport") called rhltable_lookup() to check for the duplicate transport node in transport rhashtable. But rhltable_lookup() doesn't call rcu_read_lock inside, it could cause a use-after-free issue if it tries to dereference the node that another cpu has freed it. Note that sock lock can not avoid this as it is per sock. This patch is to fix it by calling rcu_read_lock before checking for duplicate transport nodes. Fixes: cd2b70875058 ("sctp: check duplicate node before inserting a new transport") Reported-by: Andrey Konovalov <redacted> Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/sctp/input.c | 3 +++ 1 file changed, 3 insertions(+)diff --git a/net/sctp/input.c b/net/sctp/input.c index fc45896..2a28ab2 100644 --- a/net/sctp/input.c +++ b/net/sctp/input.c@@ -884,14 +884,17 @@ int sctp_hash_transport(struct sctp_transport *t) arg.paddr = &t->ipaddr; arg.lport = htons(t->asoc->base.bind_addr.port); + rcu_read_lock(); list = rhltable_lookup(&sctp_transport_hashtable, &arg, sctp_hash_params); rhl_for_each_entry_rcu(transport, tmp, list, node) if (transport->asoc->ep == t->asoc->ep) { + rcu_read_unlock(); err = -EEXIST; goto out; } + rcu_read_unlock(); err = rhltable_insert_key(&sctp_transport_hashtable, &arg, &t->node, sctp_hash_params);-- 2.1.0
Hmm, rhltable_insert_key I think returns a pointer to the existing object in the hash table if there is a collision (according to the _rhashtable_insert_fast description). Wouldn't it be better to eliminate the rhl_for_each_entry_rcu entirely, and just check the returned pointer from rhltable_insert_key? Neil