netdev - Re: [PATCH -next v2 0/3] staging: rtl8192e: Do not call kfree_skb() under spin_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1e4d321f-8252-f191-2011-043abd79a408@huawei.com>
Date: Thu, 14 Sep 2023 10:01:18 +0800
From: Ruan Jinjie <ruanjinjie@...wei.com>
To: Dan Carpenter <dan.carpenter@...aro.org>, <netdev@...r.kernel.org>
CC: <gregkh@...uxfoundation.org>, <philipp.g.hortmann@...il.com>,
	<straube.linux@...il.com>, <Larry.Finger@...inger.net>,
	<wlanfae@...ltek.com>, <mikem@...g3k.org>, <seanm@...nm.ca>,
	<linux-staging@...ts.linux.dev>
Subject: Re: [PATCH -next v2 0/3] staging: rtl8192e: Do not call kfree_skb()
 under spin_lock_irqsave()



On 2023/9/5 19:59, Dan Carpenter wrote:
> Added netdev because they're really the experts.
> 
> On Fri, Aug 25, 2023 at 09:52:10AM +0800, Jinjie Ruan wrote:
>> It is not allowed to call kfree_skb() from hardware interrupt
>> context or with interrupts being disabled.
> 
> There are no comments which say that this is not allowed.  I have
> reviewed the code to see why it's not allowed.  The only thing I can
> see is that maybe the skb->destructor(skb); in skb_release_head_state()
> sleeps?  Or possibly the uarg->callback() in skb_zcopy_clear()?

The commit e6247027e517 ("net: introduce dev_consume_skb_any()") has the
below comment:

3830 /*
3831  * It is not allowed to call kfree_skb() or consume_skb() from hardware
3832  * interrupt context or with hardware interrupts being disabled.
3833  * (in_hardirq() || irqs_disabled())
3834  *
3835  * We provide four helpers that can be used in following contexts :
3836  *
3837  * dev_kfree_skb_irq(skb) when caller drops a packet from irq context,
3838  *  replacing kfree_skb(skb)
3839  *
3840  * dev_consume_skb_irq(skb) when caller consumes a packet from irq
context.
3841  *  Typically used in place of consume_skb(skb) in TX completion path
3842  *
3843  * dev_kfree_skb_any(skb) when caller doesn't know its current irq
context,
3844  *  replacing kfree_skb(skb)
3845  *
3846  * dev_consume_skb_any(skb) when caller doesn't know its current
irq context,
3847  *  and consumed a packet. Used in place of consume_skb(skb)
3848  */

> 
> Can you comment more on why this isn't allowed?  Was this detected at
> runtime?  Do you have a stack trace?
> 
> Once I know more I can add this to Smatch so that it is detected
> automatically using static analysis.
> 
> regards,
> dan carpenter
> 
>