netdev - Re: [PATCH v5 net 6/6] net/sched: qdisc_destroy() old ingress and clsact Qdiscs before grafting

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: <7879f218-c712-e9cc-57ba-665990f5f4c9@mojatatu.com>
Date: Fri, 26 May 2023 17:21:34 -0300
From: Pedro Tammela <pctammela@...atatu.com>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: Peilin Ye <yepeilin.cs@...il.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Cong Wang <xiyou.wangcong@...il.com>, Jiri Pirko <jiri@...nulli.us>,
 Peilin Ye <peilin.ye@...edance.com>, Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>, Hillf Danton <hdanton@...a.com>,
 netdev@...r.kernel.org, Cong Wang <cong.wang@...edance.com>,
 Vlad Buslov <vladbu@...dia.com>
Subject: Re: [PATCH v5 net 6/6] net/sched: qdisc_destroy() old ingress and
 clsact Qdiscs before grafting

On 26/05/2023 16:47, Jamal Hadi Salim wrote:
> [...] Peilin, Pedro will post the new repro.

Hi!

We tweaked the reproducer to:
---
r0 = socket$netlink(0x10, 0x3, 0x0)
r1 = socket(0x10, 0x803, 0x0)
sendmsg$nl_route_sched(r1, &(0x7f0000000300)={0x0, 0x0, 
&(0x7f0000000240)={&(0x7f0000000380)=ANY=[], 0x24}}, 0x0)
getsockname$packet(r1, &(0x7f0000000200)={0x11, 0x0, <r2=>0x0, 0x1, 0x0, 
0x6, @broadcast}, &(0x7f0000000440)=0x14)
sendmsg$nl_route(r0, &(0x7f0000000040)={0x0, 0x0, 
&(0x7f0000000000)={&(0x7f0000000080)=ANY=[@ANYBLOB="480000001000050700"/20, 
@ANYRES32=r2, @ANYBLOB="0000000000000000280012000900010076657468"], 
0x48}}, 0x0)
sendmsg$nl_route_sched(0xffffffffffffffff, &(0x7f00000002c0)={0x0, 0x0, 
&(0x7f0000000280)={&(0x7f0000000540)=@...qdisc={0x30, 0x24, 0xf0b, 0x0, 
0x0, {0x0, 0x0, 0x0, r2, {}, {0xfff1, 0xffff}}, 
[@qdisc_kind_options=@...ngress={0xc}]}, 0x30}}, 0x0)
sendmsg$nl_route_sched(0xffffffffffffffff, &(0x7f0000000340)={0x0, 0x0, 
&(0x7f00000000c0)={&(0x7f0000000580)=@...tfilter={0x3c, 0x2c, 0xd27, 
0x0, 0x0, {0x0, 0x0, 0x0, r2, {}, {0xfff1, 0xffff}, {0xc}}, 
[@filter_kind_options=@...lower={{0xb}, {0xc, 0x2, 
[@TCA_FLOWER_CLASSID={0x8}]}}]}, 0x3c}}, 0x0)
r4 = socket$netlink(0x10, 0x3, 0x0)
sendmmsg(r4, &(0x7f00000002c0), 0x40000000000009f, 0x0)
r5 = socket$netlink(0x10, 0x3, 0x0)
sendmmsg(r5, &(0x7f00000002c0), 0x40000000000009f, 0x0)
---

We then generate the C program with:
syz-prog2c -sandbox none -enable net_dev -threaded -repeat 0 -prog 
peilin.syz > repro.c

Now here comes a very important detail. The above will create a new net 
namespace to shoot the netlink messages. We are only able to reproduce 
the deadlock with your patches if we comment the creation of the new 
namespace out:
---
diff --git a/repro.c b/repro.c
index ee8eb0726..5cdbfb289 100644
--- a/repro.c
+++ b/repro.c
@@ -1121,9 +1121,8 @@ static int do_sandbox_none(void)
    sandbox_common();
    drop_caps();
    initialize_netdevices_init();
-  if (unshare(CLONE_NEWNET)) {
-  }
+  // Doesn't seem to deadlock in a new netns
+  // if (unshare(CLONE_NEWNET)) {
+  // }
    write_file("/proc/sys/net/ipv4/ping_group_range", "0 65535");
    initialize_netdevices();
    setup_binderfs();

---

The reason we did this was to check on the event with 'tc mon'.
The splat is quite big, see attached. It has all the indications of a 
deadlock in the rtnl_lock.

Thanks,
Pedro
View attachment "deadlock-splat" of type "text/plain" (27730 bytes)