netdev - Re: WARNING in refcount

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

Message-ID: <CAF=yD-JR1Bg1FLat41Uua9vi1-ALyuQttChAWyWz0Yx6_01_BA@mail.gmail.com>
Date:   Thu, 19 Apr 2018 14:55:43 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     DaeRyong Jeong <threeearcat@...il.com>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Byoungyoung Lee <byoungyoung@...due.edu>,
        LKML <linux-kernel@...r.kernel.org>,
        Kyungtae Kim <kt0755@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: WARNING in refcount_dec

On Thu, Apr 19, 2018 at 2:32 AM, DaeRyong Jeong <threeearcat@...il.com> wrote:
> Hello.
> We have analyzed the cause of the crash in v4.16-rc3, WARNING in refcount_dec,
> which is found by RaceFuzzer (a modified version of Syzkaller).
>
> Since struct packet_sock's member variables, running, has_vnet_hdr, origdev
> and auxdata are declared as bitfields, accessing these variables can race if
> there is no synchronization mechanism.

Great catch.

These fields po->{running, auxdata, origdev, has_vnet_hdr} are
accessed without a uniform locking strategy.

po->running is always accessed with po->bind_lock held (with the
exception of reading in packet_seq_show, but that is best effort).

That is the only field written to outside setsockopt. If it is moved to
a separate word, it will no longer interfere with the others.

The other fields are read lockless in the various recv and send
functions, but only set in setsockopt. We've had enough
locking bugs around setsockopt that I suggest we wrap all of
those in lock_sock, like the example I gave before for
has_vnet_hdr.