netdev - Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update

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]

Message-ID: <CABdQkv9eBfsc1YnumBch4=DDn+c5sgBiNpjkyXjUfw91Wt+4cQ@mail.gmail.com>
Date:   Thu, 18 Oct 2018 10:21:48 -0300
From:   Rafael David Tinoco <rafael.tinoco@...aro.org>
To:     Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
Cc:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Jason Wang <jasowang@...hat.com>, netdev@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, tglx@...utronix.de,
        "Michael S. Tsirkin" <mst@...hat.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [RFC] virtio_net: add local_bh_disable() around u64_stats_update_begin

On Thu, Oct 18, 2018 at 6:19 AM, Toshiaki Makita
<makita.toshiaki@....ntt.co.jp> wrote:
> On 2018/10/18 18:08, Sebastian Andrzej Siewior wrote:
>> On 2018-10-18 18:00:05 [+0900], Toshiaki Makita wrote:
>>> On 2018/10/18 17:47, Sebastian Andrzej Siewior wrote:
>>>> On 2018-10-17 14:48:02 [+0800], Jason Wang wrote:
>>>>>
>>>>> On 2018/10/17 上午9:13, Toshiaki Makita wrote:
>>>>>> I'm not sure what condition triggered this warning.
>>>>
>>>> If the seqlock is acquired once in softirq and then in process context
>>>> again it is enough evidence for lockdep to trigger this warning.
>>>
>>> No. As I said that should not happen because of NAPI guard.
>> Again: lockdep saw the lock in softirq context once and in process
>> context once and this is what triggers the warning. It does not matter
>> if NAPI is enabled or not during the access in process context. If you
>> want to allow this you need further lockdep annotation…
>>
>> … but: refill_work() disables NAPI for &vi->rq[1] and refills + updates
>> stats while NAPI is enabled for &vi->rq[0].
>
> Do you mean this is false positive? rq[0] and rq[1] never race with each
> other...
>

I just came to this thread after having the same "false positive"
warning on an armhf kvm guest dmesg.

It appears to me that, at least for my case, the sequence:

u64_stats_update_begin() -> write_seqcount_begin() ->
write_seqcount_begin_nested() -> raw_write_seqcount_begin()

is only incrementing s->sequence++. With that, whenever we have:

CONFIG_TRACE_IRQ_FLAGS and CONFIG_DEBUG_LOCK_ALLOC enabled

we might face this false-positive warning since there are no locks,
but just a sequencer, right ? So, Having a barrier, after incrementing
the sequence, like I have now, won't block the other context to
"acquire" the "same lock" (not a lock for this particular case)
warning done in "seqcount_acquire()".

Hope this helps the discussion.

Link: https://bugs.linaro.org/show_bug.cgi?id=4027

Thank

Rafael Tinoco