netdev - Re: [PATCH net 2/2] wireguard: device: avoid circular netns references

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: <CAHmME9rrpr97mXLoEgk9YvZCfEUeodhCGbbWV1t2+giDBasiKQ@mail.gmail.com>
Date:   Sat, 27 Jun 2020 03:50:52 -0600
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Dmitry Vyukov <dvyukov@...gle.com>
Cc:     Netdev <netdev@...r.kernel.org>,
        syzkaller <syzkaller@...glegroups.com>
Subject: Re: [PATCH net 2/2] wireguard: device: avoid circular netns references

Hi Dmitry,

On Sat, Jun 27, 2020 at 2:59 AM Dmitry Vyukov <dvyukov@...gle.com> wrote:
> Hard to say. syzkaller frequently needs some time (days) to get
> reasonable coverage of new code.
> Is wg_netns_pre_exit executed synchronously in the context of a
> syscall? If not, then it won't be shown as covered. If yes, then what
> syscall is it?

Aw, shucks, you're right. I thought that this would be from the
syscall path for deleting a network namespace via netns_put, but
sticking a dump_stack() in there, it's clear that namespace cleanup
actually happens in a worker:

[    0.407072]  wg_netns_pre_exit+0xc/0x98
[    0.408496]  cleanup_net+0x1bd/0x2f0
[    0.409844]  process_one_work+0x17f/0x2d0
[    0.411327]  worker_thread+0x4b/0x3b0
[    0.412697]  ? rescuer_thread+0x360/0x360
[    0.414169]  kthread+0x116/0x140
[    0.415368]  ? __kthread_create_worker+0x110/0x110
[    0.417125]  ret_from_fork+0x1f/0x30

> This is related to namespaces, right? syzkaller has some descriptions
> for namespaces:
> https://github.com/google/syzkaller/blob/master/sys/linux/namespaces.txt
> But I don't know if it's enough nor I checked if they actually work.
> We have this laundry list:
> https://github.com/google/syzkaller/issues/533
> and some interns working on adding more descriptions. If you can
> identify something that's not covered but can be covered, please add
> to that list. I think we can even prioritize it then b/c most items on
> that list don't have anybody actively interested.

Something to add to that list would be:

- Create an interface of a given type (wg0->wireguard, for example)
- Move it to a namespace
- Delete a namespace
- Delete an interface

Some combination of those things might provoke issues. One would be
what the original post of this email thread was about. Another would
be that xfrmi OOPS from a few weeks ago, fixed with c95c5f58b35e
("xfrm interface: fix oops when deleting a x-netns interface"), which
has a reproducer in the commit message pretty similar to the one here.
There's an even older one in xfrmi, fixed with c5d1030f2300 ("xfrm
interface: fix list corruption for x-netns").

In short, adding and deleting namespaces and moving interfaces around
between them, amidst other syzkaller chaos, might turn up bugs,
because in the past humans have found those sorts of bugs doing the
same.

Jason