On Thu, Jun 16, 2011 at 08:36:24AM -0700, Johannes Berg wrote: > On Thu, 16 Jun 2011 15:57:08 +0530, Yogesh Ashok Powar wrote: > > Following warning was observed after the commit > > aac6af5534fade2b18682a0b9efad1a6c04c34c6 > > > >>WARNING: at net/mac80211/wpa.c:397 ccmp_encrypt_skb+0xc4/0x1f0 > > > > Consider a scenario where reserving skb tailroom is skipped > > because software encryption is not enabled. SW encryption > > can be disabled because of a) All the keys are hardware > > planted b) No key has been created. But, before actual > > transmit if hardware encryption is disabled or software > > encryption is started, there will not be enough tailroom > > space to accommodate the sw crypto's MMIC or IV and > > WARN_ON will be hit. > > > > This race between updations of hw keys and skipping & using > > tailroom, is fixed by protecting critical regions (code > > accessing crypto_tx_tailroom_needed_cnt and code from > > tailroom skipping to skb_put for IV or MMIC) with the > > spinlock. > Haha, good joke. You've got to be kidding. NACK. No inserting spinlocks > into the TX/RX paths. Using spinlocks in TX/RX path is not allowed because spinlocks will make TX/RX path to block and its illegal to block while in an RCU read-side critical section. I think, I got the joke here :(. In my opinion, following should be the fix for the races explained above a) The call synchronize_rcu() should be present before enabling sw encryption : This will make pre-existing RCU readers in RCU readside critical section to complete. Or, SKBs which have already skipped tailroom to xmit before the end of synchronize_rcu's grace period. b) Avoid tailroom skip check for RCU readside critical sections that begin inside the grace period : This will make new entrants to mandatory allocating tailroom space irrespective of sw encryption (or hw encryption with generate IV/MMIC flags set) and hence avoid the WARN_ON. Implementing first part is straight forward. For later part, RCU readers must be aware of on going grace period. I am currently not sure of any way to implement this. May be iff atomic read is allowed inside RCU read-side critical sections, then before entering the grace period, updater can set a atomic flag and unset it once done; and RCU reader can check this atomic flag to avoid the __tailroom skip check__. Please suggest your opinion. Thanks Yogesh -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
