netdev - Re: [PATCH net] veth: fix races around rq->rx_notify

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]

Date:   Tue, 15 Feb 2022 00:58:04 +0900
From:   Toshiaki Makita <toshiaki.makita1@...il.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        Toshiaki Makita <makita.toshiaki@....ntt.co.jp>,
        syzbot <syzkaller@...glegroups.com>
Subject: Re: [PATCH net] veth: fix races around rq->rx_notify_masked

On 2022/02/11 1:57, Eric Dumazet wrote:
> On Wed, Feb 9, 2022 at 4:36 AM Toshiaki Makita
> <toshiaki.makita1@...il.com> wrote:
>>
>> On 2022/02/09 8:28, Eric Dumazet wrote:
>>> From: Eric Dumazet <edumazet@...gle.com>
>>
>> Thank you for handling this case.
>>
>>> veth being NETIF_F_LLTX enabled, we need to be more careful
>>> whenever we read/write rq->rx_notify_masked.
>>>
>>> BUG: KCSAN: data-race in veth_xmit / veth_xmit
>>>
>>> w
>>> value changed: 0x00 -> 0x01
>>
>> I'm not familiar with KCSAN.
>> Does this mean rx_notify_masked value is changed while another CPU is reading it?
>>
> 
> Yes.
> 
>> If so, I'm not sure there is a problem with that.
> 
> This is a problem if not annotated properly.
> 
>> At least we could call napi_schedule() twice, but that just causes one extra napi
>> poll due to NAPIF_STATE_MISSED, and it happens very rarely?
> 
> The issue might be more problematic, a compiler might play bad games,
> look for load and store tearing.

Thank you.
I assume you mean problems like those listed in this page,
https://lwn.net/Articles/793253/
e.g. "invented stores", not exactly load/store tearing.
(since rx_notify_masked is 0/1 value and seems not able to be teared)

Now I understand that it's pretty hard to know what exact problem will happen as the 
behavior is undefined and depends heavily on compiler implementation.

Thank you for the fix again!

Toshiaki Makita