From: Arend van Spriel <arend@broadcom.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Alexander Duyck <alexander.h.duyck@intel.com>,
<netdev@vger.kernel.org>, <linux-wireless@vger.kernel.org>,
<davem@davemloft.net>, <eric.dumazet@gmail.com>,
<linville@tuxdriver.com>
Subject: Re: [PATCH net-next 2/2] mac80211: Resolve sk_refcnt/sk_wmem_alloc issue in wifi ack path
Date: Thu, 11 Sep 2014 11:38:22 +0200 [thread overview]
Message-ID: <54116D8E.20308@broadcom.com> (raw)
In-Reply-To: <1410419198.1825.5.camel@jlt4.sipsolutions.net>
On 09/11/14 09:06, Johannes Berg wrote:
> On Wed, 2014-09-10 at 18:05 -0400, Alexander Duyck wrote:
>> There is a possible issue with the use, or lack thereof of sk_refcnt and
>> sk_wmem_alloc in the wifi ack status functionality.
>>
>> Specifically if a socket were to request acknowledgements, and the socket
>> were to have sk_refcnt drop to 0 resulting in it waiting on sk_wmem_alloc
>> to reach 0 it would be possible to have sock_queue_err_skb orphan the last
>> buffer, resulting in __sk_free being called on the socket. After this the
>> buffer is enqueued on sk_error_queue, however the queue has already been
>> flushed resulting in at least a memory leak, if not a data corruption.
>
> Oh. Thanks :-)
Hi Alexander,
So why is this only an issue in wifi ack path. The sock_queue_err_skb()
does not mention the caller should hold a sock reference. This seems
entirely an issue of the sock_queue_err_skb() function itself so why not
do sk_hold/sk_put within that function. Does it impose too much overhead?
Regards,
Arend
next prev parent reply other threads:[~2014-09-11 9:38 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-10 22:04 [PATCH net-next 0/2] Address reference counting issues with sock_queue_err_skb Alexander Duyck
2014-09-10 22:05 ` [PATCH net-next v2 1/2] skb: Add documentation for skb_clone_sk Alexander Duyck
2014-09-10 22:05 ` Alexander Duyck
2014-09-10 22:05 ` [PATCH net-next 2/2] mac80211: Resolve sk_refcnt/sk_wmem_alloc issue in wifi ack path Alexander Duyck
2014-09-11 7:06 ` Johannes Berg
2014-09-11 9:38 ` Arend van Spriel [this message]
2014-09-11 14:40 ` Alexander Duyck
2014-09-11 14:40 ` Alexander Duyck
2014-09-11 15:21 ` Alexander Duyck
2014-09-11 15:53 ` Johannes Berg
2014-09-12 21:51 ` [PATCH net-next 0/2] Address reference counting issues with sock_queue_err_skb David Miller
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=54116D8E.20308@broadcom.com \
--to=arend@broadcom.com \
--cc=alexander.h.duyck@intel.com \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
--cc=netdev@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.