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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100306065655.GA14326@gondor.apana.org.au> Date: Sat, 6 Mar 2010 14:56:55 +0800 From: Herbert Xu <herbert@...dor.apana.org.au> To: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> Cc: "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org, Stephen Hemminger <shemminger@...tta.com> Subject: Re: [PATCH 6/13] bridge: Add core IGMP snooping support On Fri, Mar 05, 2010 at 09:06:56PM -0800, Paul E. McKenney wrote: > > Agreed, but the callbacks registered by the call_rcu_bh() might run > at any time, possibly quite some time after the synchronize_rcu_bh() > completes. For example, the last call_rcu_bh() might register on > one CPU, and the synchronize_rcu_bh() on another CPU. Then there > is no guarantee that the call_rcu_bh()'s callback will execute before > the synchronize_rcu_bh() returns. > > In contrast, rcu_barrier_bh() is guaranteed not to return until all > pending RCU-bh callbacks have executed. You're absolutely right. I'll send a patch to fix this. Incidentally, does rcu_barrier imply rcu_barrier_bh? What about synchronize_rcu and synchronize_rcu_bh? The reason I'm asking is that we use a mixture of rcu_read_lock_bh and rcu_read_lock all over the place but only ever use rcu_barrier and synchronize_rcu. > > I understand. However, AFAICS whatever it is that we are destroying > > is taken off the reader's visible data structure before call_rcu_bh. > > Do you have a particular case in mind where this is not the case? > > I might simply have missed the operation that removed reader > visibility, looking again... > > Ah, I see it. The "br->mdb = NULL" in br_multicast_stop() makes > it impossible for the readers to get to any of the data. Right? Yes. The read-side will see it and get nothing, while all write-side paths will see that netif_running is false and exit. > > > The br_multicast_del_pg() looks to need rcu_read_lock_bh() and > > > rcu_read_unlock_bh() around its loop, if I understand the pointer-walking > > > scheme correctly. > > > > Any function that modifies the data structure is done under the > > multicast_lock, including br_multicast_del_pg. > > But spin_lock() does not take the place of rcu_read_lock_bh(). > And so, in theory, the RCU-bh grace period could complete between > the time that br_multicast_del_pg() does its call_rcu_bh() and the > "*pp = p->next;" at the top of the next loop iteration. If so, > then br_multicast_free_pg()'s kfree() will possibly have clobbered > "p->next". Low probability, yes, but a long-running interrupt > could do the trick. > > Or is there something I am missing that is preventing an RCU-bh > grace period from completing near the bottom of br_multicast_del_pg()'s > "for" loop? Well all the locks are taken with BH disabled, this should prevent this problem, no? > > The read-side is the data path (non-IGMP multicast packets). The > > sole entry point is br_mdb_get(). > > Hmmm... So the caller is responsible for rcu_read_lock_bh()? Yes, all data paths through the bridge operate with BH disabled. > Shouldn't the br_mdb_get() code path be using hlist_for_each_entry_rcu() > in __br_mdb_ip_get(), then? Or is something else going on here? Indeed it should, I'll fix this up too. Thanks for reviewing Paul! -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@...dor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- 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