| Message ID | 1353961396-7061-1-git-send-email-arend@broadcom.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 11/26/2012 09:23 PM, Arend van Spriel wrote: > From: Piotr Haber <phaber@broadcom.com> > > In the event that tx packet can not be queued by the driver > the packet is dropped. Propagate that information to the .tx() > callback to make sure the freed packet is not accessed after > that. > > This has happened causing slab corruptions as reported by > Stanislaw Gruszka. > > Bug #47721: https://bugzilla.kernel.org/show_bug.cgi?id=47721 > > Reported-by: Stanislaw Gruszka <stf_xl@wp.pl> > Reviewed-by: Arend van Spriel <arend@broadcom.com> > Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com> > Signed-off-by: Piotr Haber <phaber@broadcom.com> > --- > drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c | 4 ++-- > drivers/net/wireless/brcm80211/brcmsmac/main.c | 14 +++++++++----- > drivers/net/wireless/brcm80211/brcmsmac/main.h | 2 +- > drivers/net/wireless/brcm80211/brcmsmac/pub.h | 2 +- > 4 files changed, 13 insertions(+), 9 deletions(-) Hi, John What is keeping you from picking up this patch? Anything I should do? This V2 removed the no-op changes in ampdu.c that Seth indicated so... please let me know. > diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c > index a744ea5..5590499 100644 > --- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c > +++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c > @@ -280,8 +280,8 @@ static void brcms_ops_tx(struct ieee80211_hw *hw, > kfree_skb(skb); > goto done; > } > - brcms_c_sendpkt_mac80211(wl->wlc, skb, hw); > - tx_info->rate_driver_data[0] = control->sta; > + if (brcms_c_sendpkt_mac80211(wl->wlc, skb, hw)) > + tx_info->rate_driver_data[0] = control->sta; This is where the slab corruption used to occur, because txinfo is part of a possibly released sk_buff. Regards, Arend > done: > spin_unlock_bh(&wl->lock); > } > diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c > index 75086b3..9fb0a4c9 100644 > --- a/drivers/net/wireless/brcm80211/brcmsmac/main.c > +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c > @@ -6095,7 +6095,7 @@ static bool brcms_c_prec_enq(struct brcms_c_info *wlc, struct pktq *q, > return brcms_c_prec_enq_head(wlc, q, pkt, prec, false); > } > > -void brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, > +bool brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, > struct sk_buff *sdu, uint prec) > { > struct brcms_txq_info *qi = wlc->pkt_queue; /* Check me */ > @@ -6110,7 +6110,9 @@ void brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, > * packet flooding from mac80211 stack > */ > brcmu_pkt_buf_free_skb(sdu); > + return false; > } > + return true; > } > > /* > @@ -7273,7 +7275,7 @@ brcms_c_d11hdrs_mac80211(struct brcms_c_info *wlc, struct ieee80211_hw *hw, > return 0; > } > > -void brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, struct sk_buff *sdu, > +bool brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, struct sk_buff *sdu, > struct ieee80211_hw *hw) > { > u8 prio; > @@ -7288,10 +7290,12 @@ void brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, struct sk_buff *sdu, > prio = ieee80211_is_data(d11_header->frame_control) ? sdu->priority : > MAXPRIO; > fifo = prio2fifo[prio]; > - if (brcms_c_d11hdrs_mac80211(wlc, hw, sdu, scb, 0, 1, fifo, 0)) > - return; > - brcms_c_txq_enq(wlc, scb, sdu, BRCMS_PRIO_TO_PREC(prio)); > + brcms_c_d11hdrs_mac80211(wlc, hw, sdu, scb, 0, 1, fifo, 0); > + if (!brcms_c_txq_enq(wlc, scb, sdu, BRCMS_PRIO_TO_PREC(prio))) > + return false; > brcms_c_send_q(wlc); > + > + return true; > } > > void brcms_c_send_q(struct brcms_c_info *wlc) > diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.h b/drivers/net/wireless/brcm80211/brcmsmac/main.h > index 8debc74..b44725c 100644 > --- a/drivers/net/wireless/brcm80211/brcmsmac/main.h > +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.h > @@ -642,7 +642,7 @@ extern void brcms_c_txfifo(struct brcms_c_info *wlc, uint fifo, > bool commit, s8 txpktpend); > extern void brcms_c_txfifo_complete(struct brcms_c_info *wlc, uint fifo, > s8 txpktpend); > -extern void brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, > +extern bool brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, > struct sk_buff *sdu, uint prec); > extern void brcms_c_print_txstatus(struct tx_status *txs); > extern int brcms_b_xmtfifo_sz_get(struct brcms_hardware *wlc_hw, uint fifo, > diff --git a/drivers/net/wireless/brcm80211/brcmsmac/pub.h b/drivers/net/wireless/brcm80211/brcmsmac/pub.h > index 5855f4f..bfa2630 100644 > --- a/drivers/net/wireless/brcm80211/brcmsmac/pub.h > +++ b/drivers/net/wireless/brcm80211/brcmsmac/pub.h > @@ -321,7 +321,7 @@ extern void brcms_c_intrsrestore(struct brcms_c_info *wlc, u32 macintmask); > extern bool brcms_c_intrsupd(struct brcms_c_info *wlc); > extern bool brcms_c_isr(struct brcms_c_info *wlc, bool *wantdpc); > extern bool brcms_c_dpc(struct brcms_c_info *wlc, bool bounded); > -extern void brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, > +extern bool brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, > struct sk_buff *sdu, > struct ieee80211_hw *hw); > extern bool brcms_c_aggregatable(struct brcms_c_info *wlc, u8 tid); > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arend, "Arend van Spriel" <arend@broadcom.com> writes: > What is keeping you from picking up this patch? Anything I should do? > This V2 removed the no-op changes in ampdu.c that Seth indicated so... Most likely it's too late. Dave already told that that John's previous pull request might not make it to 3.7.
On Mon, Dec 03, 2012 at 04:24:31PM +0200, Kalle Valo wrote: > Hi Arend, > > "Arend van Spriel" <arend@broadcom.com> writes: > > > What is keeping you from picking up this patch? Anything I should do? > > This V2 removed the no-op changes in ampdu.c that Seth indicated so... > > Most likely it's too late. Dave already told that that John's previous > pull request might not make it to 3.7. Yes, that. Also, given the merge difficulties this creates and the fact that Seth's patch is already queued for 3.8, maybe you should consider sending this for the 3.7 stable series when it opens? John
On 12/03/2012 07:39 PM, John W. Linville wrote: > On Mon, Dec 03, 2012 at 04:24:31PM +0200, Kalle Valo wrote: >> Hi Arend, >> >> "Arend van Spriel" <arend@broadcom.com> writes: >> >>> What is keeping you from picking up this patch? Anything I should do? >>> This V2 removed the no-op changes in ampdu.c that Seth indicated so... >> >> Most likely it's too late. Dave already told that that John's previous >> pull request might not make it to 3.7. > > Yes, that. Also, given the merge difficulties this creates and the > fact that Seth's patch is already queued for 3.8, maybe you should > consider sending this for the 3.7 stable series when it opens? > > John > That was my plan when it would not get in for 3.7.0. I wanted it to go to stable anyway as the fix is also needed for 3.6.x stable. Just wanted to get it upstream fixed first. For 3.8 the fix has been reworked on top of Seth patches and is in wireless-next (c4dea35e brcmsmac: handle packet drop during transmit correctly). Should I refer to that commit when submitting to stable (although it is a slightly different patch). Gr. AvS -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 03, 2012 at 07:54:54PM +0100, Arend van Spriel wrote: > On 12/03/2012 07:39 PM, John W. Linville wrote: > > On Mon, Dec 03, 2012 at 04:24:31PM +0200, Kalle Valo wrote: > >> Hi Arend, > >> > >> "Arend van Spriel" <arend@broadcom.com> writes: > >> > >>> What is keeping you from picking up this patch? Anything I should do? > >>> This V2 removed the no-op changes in ampdu.c that Seth indicated so... > >> > >> Most likely it's too late. Dave already told that that John's previous > >> pull request might not make it to 3.7. > > > > Yes, that. Also, given the merge difficulties this creates and the > > fact that Seth's patch is already queued for 3.8, maybe you should > > consider sending this for the 3.7 stable series when it opens? > > > > John > > > > That was my plan when it would not get in for 3.7.0. I wanted it to go > to stable anyway as the fix is also needed for 3.6.x stable. Just wanted > to get it upstream fixed first. > > For 3.8 the fix has been reworked on top of Seth patches and is in > wireless-next (c4dea35e brcmsmac: handle packet drop during transmit > correctly). Should I refer to that commit when submitting to stable > (although it is a slightly different patch). Yes, I think the stable team will appreciate that. Thanks, John
On 12/03/2012 07:59 PM, John W. Linville wrote: > On Mon, Dec 03, 2012 at 07:54:54PM +0100, Arend van Spriel wrote: >> On 12/03/2012 07:39 PM, John W. Linville wrote: >>> On Mon, Dec 03, 2012 at 04:24:31PM +0200, Kalle Valo wrote: >>>> Hi Arend, >>>> >>>> "Arend van Spriel" <arend@broadcom.com> writes: >>>> >>>>> What is keeping you from picking up this patch? Anything I should do? >>>>> This V2 removed the no-op changes in ampdu.c that Seth indicated so... >>>> >>>> Most likely it's too late. Dave already told that that John's previous >>>> pull request might not make it to 3.7. >>> >>> Yes, that. Also, given the merge difficulties this creates and the >>> fact that Seth's patch is already queued for 3.8, maybe you should >>> consider sending this for the 3.7 stable series when it opens? >>> >>> John >>> >> >> That was my plan when it would not get in for 3.7.0. I wanted it to go >> to stable anyway as the fix is also needed for 3.6.x stable. Just wanted >> to get it upstream fixed first. >> >> For 3.8 the fix has been reworked on top of Seth patches and is in >> wireless-next (c4dea35e brcmsmac: handle packet drop during transmit >> correctly). Should I refer to that commit when submitting to stable >> (although it is a slightly different patch). > > Yes, I think the stable team will appreciate that. > > Thanks, > > John > Hi John, Not keeping a close eye on LKML, but I noticed 3.7-rc8 is out. Would you reconsider taking this patch for 3.7? Would be nice to be rid of slab corruption in v3.7.0. Gr. AvS -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c index a744ea5..5590499 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c +++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c @@ -280,8 +280,8 @@ static void brcms_ops_tx(struct ieee80211_hw *hw, kfree_skb(skb); goto done; } - brcms_c_sendpkt_mac80211(wl->wlc, skb, hw); - tx_info->rate_driver_data[0] = control->sta; + if (brcms_c_sendpkt_mac80211(wl->wlc, skb, hw)) + tx_info->rate_driver_data[0] = control->sta; done: spin_unlock_bh(&wl->lock); } diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c index 75086b3..9fb0a4c9 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/main.c +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c @@ -6095,7 +6095,7 @@ static bool brcms_c_prec_enq(struct brcms_c_info *wlc, struct pktq *q, return brcms_c_prec_enq_head(wlc, q, pkt, prec, false); } -void brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, +bool brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, struct sk_buff *sdu, uint prec) { struct brcms_txq_info *qi = wlc->pkt_queue; /* Check me */ @@ -6110,7 +6110,9 @@ void brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, * packet flooding from mac80211 stack */ brcmu_pkt_buf_free_skb(sdu); + return false; } + return true; } /* @@ -7273,7 +7275,7 @@ brcms_c_d11hdrs_mac80211(struct brcms_c_info *wlc, struct ieee80211_hw *hw, return 0; } -void brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, struct sk_buff *sdu, +bool brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, struct sk_buff *sdu, struct ieee80211_hw *hw) { u8 prio; @@ -7288,10 +7290,12 @@ void brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, struct sk_buff *sdu, prio = ieee80211_is_data(d11_header->frame_control) ? sdu->priority : MAXPRIO; fifo = prio2fifo[prio]; - if (brcms_c_d11hdrs_mac80211(wlc, hw, sdu, scb, 0, 1, fifo, 0)) - return; - brcms_c_txq_enq(wlc, scb, sdu, BRCMS_PRIO_TO_PREC(prio)); + brcms_c_d11hdrs_mac80211(wlc, hw, sdu, scb, 0, 1, fifo, 0); + if (!brcms_c_txq_enq(wlc, scb, sdu, BRCMS_PRIO_TO_PREC(prio))) + return false; brcms_c_send_q(wlc); + + return true; } void brcms_c_send_q(struct brcms_c_info *wlc) diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.h b/drivers/net/wireless/brcm80211/brcmsmac/main.h index 8debc74..b44725c 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/main.h +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.h @@ -642,7 +642,7 @@ extern void brcms_c_txfifo(struct brcms_c_info *wlc, uint fifo, bool commit, s8 txpktpend); extern void brcms_c_txfifo_complete(struct brcms_c_info *wlc, uint fifo, s8 txpktpend); -extern void brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, +extern bool brcms_c_txq_enq(struct brcms_c_info *wlc, struct scb *scb, struct sk_buff *sdu, uint prec); extern void brcms_c_print_txstatus(struct tx_status *txs); extern int brcms_b_xmtfifo_sz_get(struct brcms_hardware *wlc_hw, uint fifo, diff --git a/drivers/net/wireless/brcm80211/brcmsmac/pub.h b/drivers/net/wireless/brcm80211/brcmsmac/pub.h index 5855f4f..bfa2630 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/pub.h +++ b/drivers/net/wireless/brcm80211/brcmsmac/pub.h @@ -321,7 +321,7 @@ extern void brcms_c_intrsrestore(struct brcms_c_info *wlc, u32 macintmask); extern bool brcms_c_intrsupd(struct brcms_c_info *wlc); extern bool brcms_c_isr(struct brcms_c_info *wlc, bool *wantdpc); extern bool brcms_c_dpc(struct brcms_c_info *wlc, bool bounded); -extern void brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, +extern bool brcms_c_sendpkt_mac80211(struct brcms_c_info *wlc, struct sk_buff *sdu, struct ieee80211_hw *hw); extern bool brcms_c_aggregatable(struct brcms_c_info *wlc, u8 tid);