netdev - Re: [PATCH net v2] net: sched: add barrier to ensure correct ordering for lockless qdisc

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20210621162948.179f0753@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 21 Jun 2021 16:29:48 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Yunsheng Lin <yunshenglin0825@...il.com>
Cc:     Yunsheng Lin <linyunsheng@...wei.com>, davem@...emloft.net,
        olteanv@...il.com, ast@...nel.org, daniel@...earbox.net,
        andriin@...com, edumazet@...gle.com, weiwan@...gle.com,
        cong.wang@...edance.com, ap420073@...il.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        linuxarm@...neuler.org, mkl@...gutronix.de,
        linux-can@...r.kernel.org, jhs@...atatu.com,
        xiyou.wangcong@...il.com, jiri@...nulli.us, andrii@...nel.org,
        kafai@...com, songliubraving@...com, yhs@...com,
        john.fastabend@...il.com, kpsingh@...nel.org, bpf@...r.kernel.org,
        jonas.bonn@...rounds.com, pabeni@...hat.com, mzhivich@...mai.com,
        johunt@...mai.com, albcamus@...il.com, kehuan.feng@...il.com,
        a.fatoum@...gutronix.de, atenart@...nel.org,
        alexander.duyck@...il.com, hdanton@...a.com, jgross@...e.com,
        JKosina@...e.com, mkubecek@...e.cz, bjorn@...nel.org,
        alobakin@...me
Subject: Re: [PATCH net v2] net: sched: add barrier to ensure correct
 ordering for lockless qdisc

On Sat, 19 Jun 2021 10:30:09 +0000 Yunsheng Lin wrote:
> When debugging pointed to the misordering between STATE_MISSED
> setting/clearing and STATE_MISSED checking, only _after_atomic()
> was added first, and it did not fix the misordering problem,
> when both _before_atomic() and _after_atomic() were added, the
> misordering problem disappeared.
> 
> I suppose _before_atomic() matters because the STATE_MISSED
> setting and the lock rechecking is only done when first check of
> STATE_MISSED returns false. _before_atomic() is used to make sure
> the first check returns correct result, if it does not return the
> correct result, then we may have misordering problem too.
> 
>      cpu0                        cpu1
>                               clear MISSED
>                              _after_atomic()
>                                 dequeue
>     enqueue
>  first trylock() #false
>   MISSED check #*true* ?
> 
> As above, even cpu1 has a _after_atomic() between clearing
> STATE_MISSED and dequeuing, we might stiil need a barrier to
> prevent cpu0 doing speculative MISSED checking before cpu1
> clearing MISSED?
> 
> And the implicit load-acquire barrier contained in the first
> trylock() does not seems to prevent the above case too.
> 
> And there is no load-acquire barrier in pfifo_fast_dequeue()
> too, which possibly make the above case more likely to happen.

Ah, you're right. The test_bit() was not in the patch context, 
I forgot it's there... Both barriers are indeed needed.