netdev - Re: [PATCH net v4 1/1] net: sched: fix ets qdisc OOB Indexing

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: <87zfjvqa6w.fsf@nvidia.com>
Date: Mon, 13 Jan 2025 11:21:29 +0100
From: Petr Machata <petrm@...dia.com>
To: Cong Wang <xiyou.wangcong@...il.com>
CC: Jamal Hadi Salim <jhs@...atatu.com>, <netdev@...r.kernel.org>,
	<jiri@...nulli.us>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <petrm@...lanox.com>, <security@...nel.org>,
	<g1042620637@...il.com>
Subject: Re: [PATCH net v4 1/1] net: sched: fix ets qdisc OOB Indexing


Cong Wang <xiyou.wangcong@...il.com> writes:

> On Sat, Jan 11, 2025 at 09:57:39AM -0500, Jamal Hadi Salim wrote:
>> diff --git a/net/sched/sch_ets.c b/net/sched/sch_ets.c
>> index f80bc05d4c5a..516038a44163 100644
>> --- a/net/sched/sch_ets.c
>> +++ b/net/sched/sch_ets.c
>> @@ -91,6 +91,8 @@ ets_class_from_arg(struct Qdisc *sch, unsigned long arg)
>>  {
>>  	struct ets_sched *q = qdisc_priv(sch);
>>  
>> +	if (arg == 0 || arg > q->nbands)
>> +		return NULL;
>>  	return &q->classes[arg - 1];
>>  }
>
> I must miss something here. Some callers of this function don't handle
> NULL at all, so are you sure it is safe to return NULL for all the
> callers here??
>
> For one quick example:
>
> 322 static int ets_class_dump_stats(struct Qdisc *sch, unsigned long arg,
> 323                                 struct gnet_dump *d)
> 324 {
> 325         struct ets_class *cl = ets_class_from_arg(sch, arg);
> 326         struct Qdisc *cl_q = cl->qdisc;
>
> 'cl' is not checked against NULL before dereferencing it.
>
> There are other cases too, please ensure _all_ of them handle NULL
> correctly.

Yeah, I looked through ets_class_from_arg() callers last week and I
think that besides the one call that needs patching, which already
handles NULL, in all other cases the arg passed to ets_class_from_arg()
comes from class_find, and therefore shouldn't cause the NULL return.