netdev - Re: [PATCH] net: disable irq in napi

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Feb 2023 14:36:46 +0800
From:   Kassey Li <quic_yingangl@...cinc.com>
To:     Eric Dumazet <edumazet@...gle.com>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: disable irq in napi_poll



On 2/15/2023 4:32 PM, Eric Dumazet wrote:
> On Wed, Feb 15, 2023 at 8:20 AM Kassey Li <quic_yingangl@...cinc.com> wrote:
>>
>> There is list_del action in napi_poll, fix race condition by
>> disable irq.
>>
>> similar report:
>> https://syzkaller.appspot.com/bug?id=309955e7f02812d7bfb828c22b517349d9f068
>> bc
>>
>> list_del corruption. next->prev should be ffffff88ea0bd4c0, but was
>> ffffff8a787099c0
>> ------------[ cut here ]------------
>> kernel BUG at lib/list_debug.c:56!
>>
>> pstate: 62400005 (nZCv daif +PAN -UAO +TCO -DIT -SSBS BTYPE=--)
>> pc : __list_del_entry_valid+0xa8/0xac
>> lr : __list_del_entry_valid+0xa8/0xac
>> sp : ffffffc0081bbce0
>> x29: ffffffc0081bbce0 x28: 0000000000000018 x27: 0000000000000059
>> x26: ffffffef130c6040 x25: ffffffc0081bbcf0 x24: ffffffc0081bbd00
>> x23: 000000010003d37f x22: 000000000000012c x21: ffffffef130c9000
>> x20: ffffff8a786cf9c0 x19: ffffff88ea0bd4c0 x18: ffffffc00816d030
>> x17: ffffffffffffffff x16: 0000000000000004 x15: 0000000000000004
>> x14: ffffffef131bae30 x13: 0000000000002b84 x12: 0000000000000003
>> x11: 0000000100002b84 x10: c000000100002b84 x9 : 1f2ede939758e700
>> x8 : 1f2ede939758e700 x7 : 205b5d3330383232 x6 : 302e33303331205b
>> x5 : ffffffef13750358 x4 : ffffffc0081bb9df x3 : 0000000000000000
>> x2 : ffffff8a786be9c8 x1 : 0000000000000000 x0 : 000000000000007c
>>
>> Call trace:
>> __list_del_entry_valid+0xa8/0xac
>> net_rx_action+0xfc/0x3a0
>> _stext+0x174/0x5f4
>> run_ksoftirqd+0x34/0x74
>> smpboot_thread_fn+0x1d8/0x464
>> kthread+0x168/0x1dc
>> ret_from_fork+0x10/0x20
>> Code: d4210000 f000cbc0 91161000 97de537a (d4210000)
>> ---[ end trace 8b3858d55ee59b7c ]---
>> Kernel panic - not syncing: Oops - BUG: Fatal exception in interrupt
>>
>> Signed-off-by: Kassey Li <quic_yingangl@...cinc.com>
> 
> No tags ?
   hi Eric, can you share a example of "tags" ?  sorry I'm not aware of 
this.
> 
>> ---
>>   net/core/dev.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index b76fb37b381e..0c677a563232 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -6660,7 +6660,9 @@ static __latent_entropy void net_rx_action(struct softirq_action *h)
>>                  }
>>
>>                  n = list_first_entry(&list, struct napi_struct, poll_list);
>> +               local_irq_disable();
>>                  budget -= napi_poll(n, &repoll);
>> +               local_irq_enable();
>>
>>                  /* If softirq window is exhausted then punt.
>>                   * Allow this to run for 2 jiffies since which will allow
>> --
>> 2.17.1
>>
> 
> Absolutely not.
> 
> NAPI runs in softirq mode, not hard irq.
> 
> You will have to spend more time finding the real bug.

   without this patch, issue is easy to reproduced when do phone clone( 
a new phone and old phone to clone data/app).
   with this path, issue is not seen.

   in the function of net_rx_action, list init/add is go with 
local_irq_disable/enable, however, napi_poll will call into list_del.
  there is no such local_irq_disable/enable.

   may you give some suggest on this to further narrow down ?