From: "Arend van Spriel" <arend@broadcom.com>
To: "John W. Linville" <linville@tuxdriver.com>
Cc: "Linux Wireless List" <linux-wireless@vger.kernel.org>,
"Piotr Haber" <phaber@broadcom.com>,
"Seth Forshee" <seth.forshee@canonical.com>
Subject: Re: [PATCH V2 v3.7] brcmsmac: handle packet drop on enqueuing correctly
Date: Mon, 3 Dec 2012 09:33:41 +0100 [thread overview]
Message-ID: <50BC63E5.9020101@broadcom.com> (raw)
In-Reply-To: <1353961396-7061-1-git-send-email-arend@broadcom.com>
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);
>
next prev parent reply other threads:[~2012-12-03 8:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-26 20:23 [PATCH V2 v3.7] brcmsmac: handle packet drop on enqueuing correctly Arend van Spriel
2012-12-03 8:33 ` Arend van Spriel [this message]
2012-12-03 14:24 ` Kalle Valo
2012-12-03 18:39 ` John W. Linville
2012-12-03 18:54 ` Arend van Spriel
2012-12-03 18:59 ` John W. Linville
2012-12-04 19:52 ` Arend van Spriel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50BC63E5.9020101@broadcom.com \
--to=arend@broadcom.com \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=phaber@broadcom.com \
--cc=seth.forshee@canonical.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).