netdev - Re: [PATCH net-n] net/sched: stop qdisc_tree_reduce_backlog on TC_H

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: <9ce50453-c78a-4883-a888-51e31f673f40@mojatatu.com>
Date: Mon, 28 Oct 2024 11:36:00 -0300
From: Pedro Tammela <pctammela@...atatu.com>
To: Cong Wang <xiyou.wangcong@...il.com>, Jamal Hadi Salim <jhs@...atatu.com>
Cc: netdev@...r.kernel.org, markovicbudimir@...il.com, victor@...atatu.com,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, jiri@...nulli.us
Subject: Re: [PATCH net-n] net/sched: stop qdisc_tree_reduce_backlog on
 TC_H_ROOT

On 26/10/2024 13:47, Cong Wang wrote:
> On Thu, Oct 24, 2024 at 12:55:47PM -0400, Jamal Hadi Salim wrote:
>> From: Pedro Tammela <pctammela@...atatu.com>
>>
>> In qdisc_tree_reduce_backlog, Qdiscs with major handle ffff: are assumed
>> to be either root or ingress. This assumption is bogus since it's valid
>> to create egress qdiscs with major handle ffff:
>> Budimir Markovic found that for qdiscs like DRR that maintain an active
>> class list, it will cause a UAF with a dangling class pointer.
>>
>> In 066a3b5b2346, the concern was to avoid iterating over the ingress
>> qdisc since its parent is itself. The proper fix is to stop when parent
>> TC_H_ROOT is reached because the only way to retrieve ingress is when a
>> hierarchy which does not contain a ffff: major handle call into
>> qdisc_lookup with TC_H_MAJ(TC_H_ROOT).
>>
>> In the scenario where major ffff: is an egress qdisc in any of the tree
>> levels, the updates will also propagate to TC_H_ROOT, which then the
>> iteration must stop.
>>
>> Fixes: 066a3b5b2346 ("[NET_SCHED] sch_api: fix qdisc_tree_decrease_qlen() loop")
>> Reported-by: Budimir Markovic <markovicbudimir@...il.com>
>> Suggested-by: Jamal Hadi Salim <jhs@...atatu.com>
>> Tested-by: Victor Nogueira <victor@...atatu.com>
>> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
>> Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
>>
>>   net/sched/sch_api.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Can we also add a selftest since it is reproducible?

Yep, we are just waiting for it to be in net-next so we don't crash 
downstream CIs