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

[Cong Wang <xiyou.wangcong@gmail.com>]

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!