netdev - Re: [PATCH] net/sched: fix use-after-free in taprio_dev

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: <CANn89i+Lp5n-+TQHLg1=1FauDt45w0P3mneZaiWD7gRnFesVpg@mail.gmail.com>
Date: Wed, 11 Jun 2025 04:28:44 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Hyunwoo Kim <imv4bel@...il.com>
Cc: Vladimir Oltean <vladimir.oltean@....com>, vinicius.gomes@...el.com, jhs@...atatu.com, 
	xiyou.wangcong@...il.com, jiri@...nulli.us, davem@...emloft.net, 
	kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, netdev@...r.kernel.org, 
	v4bel@...ori.io
Subject: Re: [PATCH] net/sched: fix use-after-free in taprio_dev_notifier

On Wed, Jun 11, 2025 at 4:24 AM Hyunwoo Kim <imv4bel@...il.com> wrote:
>
> On Wed, Jun 11, 2025 at 04:01:50AM -0700, Eric Dumazet wrote:
> > On Wed, Jun 11, 2025 at 3:03 AM Hyunwoo Kim <imv4bel@...il.com> wrote:
> > >
> > > Since taprio’s taprio_dev_notifier() isn’t protected by an
> > > RCU read-side critical section, a race with advance_sched()
> > > can lead to a use-after-free.
> > >
> > > Adding rcu_read_lock() inside taprio_dev_notifier() prevents this.
> > >
> > > Signed-off-by: Hyunwoo Kim <imv4bel@...il.com>
> >
> > Looks good to me, but we need a Fixes: tag and/or a CC: stable@ o make
> > sure this patch reaches appropriate stable trees.
>
> Understood. I will submit the v2 patch after adding the tags.

Thanks, please wait ~24 hours (as described in
Documentation/process/maintainer-netdev.rst )

>
> >
> > Also please CC the author of the  patch.
>
> Does “CC” here refer to a patch tag, or to the email’s cc? And by
> “patch author” you mean the author of the patch
> fed87cc6718ad5f80aa739fee3c5979a8b09d3a6, right?

Exactly. Blamed patch author.

>
> >
> > It seems bug came with
> >
> > commit fed87cc6718ad5f80aa739fee3c5979a8b09d3a6
> > Author: Vladimir Oltean <vladimir.oltean@....com>
> > Date:   Tue Feb 7 15:54:38 2023 +0200
> >
> >     net/sched: taprio: automatically calculate queueMaxSDU based on TC
> > gate durations
> >
> >
> >
> >
> > > ---
> > >  net/sched/sch_taprio.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> > > index 14021b812329..bd2b02d1dc63 100644
> > > --- a/net/sched/sch_taprio.c
> > > +++ b/net/sched/sch_taprio.c
> > > @@ -1320,6 +1320,7 @@ static int taprio_dev_notifier(struct notifier_block *nb, unsigned long event,
> > >         if (event != NETDEV_UP && event != NETDEV_CHANGE)
> > >                 return NOTIFY_DONE;
> > >
> > > +       rcu_read_lock();
> > >         list_for_each_entry(q, &taprio_list, taprio_list) {
> > >                 if (dev != qdisc_dev(q->root))
> > >                         continue;
> > > @@ -1328,16 +1329,17 @@ static int taprio_dev_notifier(struct notifier_block *nb, unsigned long event,
> > >
> > >                 stab = rtnl_dereference(q->root->stab);
> > >
> > > -               oper = rtnl_dereference(q->oper_sched);
> > > +               oper = rcu_dereference(q->oper_sched);
> > >                 if (oper)
> > >                         taprio_update_queue_max_sdu(q, oper, stab);
> > >
> > > -               admin = rtnl_dereference(q->admin_sched);
> > > +               admin = rcu_dereference(q->admin_sched);
> > >                 if (admin)
> > >                         taprio_update_queue_max_sdu(q, admin, stab);
> > >
> > >                 break;
> > >         }
> > > +       rcu_read_unlock();
> > >
> > >         return NOTIFY_DONE;
> > >  }
> > > --
> > > 2.34.1
> > >