From: Stanislaw Gruszka <sgruszka@redhat.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
linux-wireless@vger.kernel.org
Subject: Re: [PATCH 1/2] mac80211: fix calling ieee80211_free_txskb with NULL skb
Date: Wed, 19 Feb 2014 15:48:31 +0100 [thread overview]
Message-ID: <20140219144831.GF1851@redhat.com> (raw)
In-Reply-To: <20140219132134.GD1851@redhat.com>
On Wed, Feb 19, 2014 at 02:21:35PM +0100, Stanislaw Gruszka wrote:
> > In any case, while this solves the crash which is a good thing, it still
> > leaves the code buggy. This crash seems to occur in the following racy
> > scenario:
> >
> > * station is sleeping
> > * frame TX to station begins
> > * station wakes up
> > * frame TX goes into the queue length check, finds long queue
> > * pending frames are transmitted
> > * queue is now empty
> > * old = skb_dequeue() returns NULL
> > * *kaboom*
> >
> > The problem is that you're just fixing the "*kaboom*" part, so the code
> > will continue like this:
> >
> > * old is NULL
> > * no kaboom
> > * new frame is queued on ps_tx_buf queue
> > * frame never gets transmitted
>
> When started to look at that code I found at least 3 bugs, but miss
> this one :-)
>
> Why frame will not be transmitted, we are disabling PS, but buffers
> stays not empty ?
Ok, I think I see this, it seems to be race condition in
ieee80211_sta_ps_deliver_wakeup().
Perhaps it could be solved by modifying
ieee80211_add_pending_skbs_fn() to take list of queues as argument,
that function seems properly stop queues, add buffered frames to
pending queue, clear WLAN_STA_PS_STA and then wake up queues. Or
just stop using ieee80211_add_pending_skbs_fn() and do the same
sequence directly on ieee80211_sta_ps_deliver_wakeup() .
Stanislaw
next prev parent reply other threads:[~2014-02-19 14:46 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-19 12:28 [PATCH 1/2] mac80211: fix calling ieee80211_free_txskb with NULL skb Stanislaw Gruszka
2014-02-19 12:28 ` [PATCH 2/2] mac80211: protect skb_queue_len(&ps->bc_buf) by lock Stanislaw Gruszka
2014-02-19 13:14 ` Johannes Berg
2014-02-19 13:35 ` Stanislaw Gruszka
2014-02-19 14:51 ` Johannes Berg
2014-02-19 15:09 ` Stanislaw Gruszka
2014-02-19 16:36 ` Johannes Berg
2014-02-20 7:56 ` Stanislaw Gruszka
2014-02-20 7:59 ` Johannes Berg
2014-02-20 8:17 ` Stanislaw Gruszka
2014-02-19 12:33 ` [PATCH 1/2] mac80211: fix calling ieee80211_free_txskb with NULL skb Johannes Berg
2014-02-19 12:39 ` Grumbach, Emmanuel
2014-02-19 12:46 ` Grumbach, Emmanuel
2014-02-19 13:21 ` Stanislaw Gruszka
2014-02-19 14:48 ` Stanislaw Gruszka [this message]
2014-02-19 14:50 ` Johannes Berg
2014-02-19 15:00 ` Stanislaw Gruszka
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=20140219144831.GF1851@redhat.com \
--to=sgruszka@redhat.com \
--cc=emmanuel.grumbach@intel.com \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.