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] [day] [month] [year] [list]
Message-ID: <CAM0EoMkvOOgT-SU1A7=29Tz1JrqpO7eDsoQAXQsYjCGds=1C-w@mail.gmail.com>
Date: Mon, 13 Jan 2025 06:47:02 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Petr Machata <petrm@...dia.com>
Cc: Cong Wang <xiyou.wangcong@...il.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

On Mon, Jan 13, 2025 at 5:29 AM Petr Machata <petrm@...dia.com> wrote:
>
>
> 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.

Exactly.
Regardless - once the nodes are created we are guaranteed non-null.
See other qdiscs, not just ets.

cheers,
jamal

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ