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: <Z4RWFNIvS31kVhvA@pop-os.localdomain>
Date: Sun, 12 Jan 2025 15:53:56 -0800
From: Cong Wang <xiyou.wangcong@...il.com>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: 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

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.

Thanks!