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: <ZHE8P9Bi6FlKz4US@C02FL77VMD6R.googleapis.com>
Date: Fri, 26 May 2023 16:09:51 -0700
From: Peilin Ye <yepeilin.cs@...il.com>
To: Pedro Tammela <pctammela@...atatu.com>,
	Jamal Hadi Salim <jhs@...atatu.com>
Cc: Jamal Hadi Salim <jhs@...atatu.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 Fri, May 26, 2023 at 05:21:34PM -0300, Pedro Tammela wrote:
> On 26/05/2023 16:47, Jamal Hadi Salim wrote:
> > [...] Peilin, Pedro will post the new repro.
> 
> 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 a lot, I'll get right on it.

Peilin Ye