lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 21 Jan 2015 16:51:05 +0800 From: Ying Xue <ying.xue@...driver.com> To: Thomas Graf <tgraf@...g.ch>, "richard.alpe@...csson.com >> Richard Alpe" <richard.alpe@...csson.com> CC: <davem@...emloft.net>, Netdev <netdev@...r.kernel.org>, <tipc-discussion@...ts.sourceforge.net> Subject: Re: [PATCH net-next] rhashtable: rhashtable_remove() must unlink in both tbl and future_tbl Hi Thomas, I will be on vacation in next few days. So if you have an updated patch resolving the issue, please let Richard(richard.alpe@...csson.com) help you to verify because he initially found the issue. After I identified what it closely was related to rhashtable and it also happened in netlink, I posted it on netdev mail list. Regards, Ying On 01/21/2015 12:58 AM, Thomas Graf wrote: > As removals can occur during resizes, entries may be referred to from > both tbl and future_tbl when the removal is requested. Therefore > rhashtable_remove() must unlink the entry in both tables if this is > the case. The existing code did search both tables but stopped when it > hit the first match. > > Failing to do so resulted in use after remove. > > Fixes: 97defe1 ("rhashtable: Per bucket locks & deferred expansion/shrinking") > Reported-by: Ying Xue <ying.xue@...driver.com> > Signed-off-by: Thomas Graf <tgraf@...g.ch> > --- > > Ying: This should fix the panic that was at the end of your TIPC > related boot log. I'm still working on the use after free. > > lib/rhashtable.c | 24 +++++++++++++++--------- > 1 file changed, 15 insertions(+), 9 deletions(-) > > diff --git a/lib/rhashtable.c b/lib/rhashtable.c > index a4449c4..b1aa10e 100644 > --- a/lib/rhashtable.c > +++ b/lib/rhashtable.c > @@ -592,6 +592,7 @@ bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj) > struct rhash_head *he; > spinlock_t *lock; > unsigned int hash; > + bool ret = false; > > rcu_read_lock(); > tbl = rht_dereference_rcu(ht->tbl, ht); > @@ -609,17 +610,16 @@ restart: > } > > rcu_assign_pointer(*pprev, obj->next); > - atomic_dec(&ht->nelems); > - > - spin_unlock_bh(lock); > - > - rhashtable_wakeup_worker(ht); > - > - rcu_read_unlock(); > > - return true; > + ret = true; > + break; > } > > + /* The entry may be linked in either 'tbl', 'future_tbl', or both. > + * 'future_tbl' only exists for a short period of time during > + * resizing. Thus traversing both is fine and the added cost is > + * very rare. > + */ > if (tbl != rht_dereference_rcu(ht->future_tbl, ht)) { > spin_unlock_bh(lock); > > @@ -632,9 +632,15 @@ restart: > } > > spin_unlock_bh(lock); > + > + if (ret) { > + atomic_dec(&ht->nelems); > + rhashtable_wakeup_worker(ht); > + } > + > rcu_read_unlock(); > > - return false; > + return ret; > } > EXPORT_SYMBOL_GPL(rhashtable_remove); > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists