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: <aElqlbPP+9UcInJa@v4bel-B760M-AORUS-ELITE-AX>
Date: Wed, 11 Jun 2025 07:37:57 -0400
From: Hyunwoo Kim <imv4bel@...il.com>
To: Eric Dumazet <edumazet@...gle.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,
	imv4bel@...il.com
Subject: Re: [PATCH] net/sched: fix use-after-free in taprio_dev_notifier

On Wed, Jun 11, 2025 at 04:28:44AM -0700, Eric Dumazet wrote:
> 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 )

Okay, I will submit the v2 patch tomorrow. Thank you for reviewing it!

> 
> >
> > >
> > > 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.

To avoid confusion: when you say “CC the patch author,” do you mean 
adding the author to the CC tag in the v2 patch, or simply including 
them in the email’s CC field?
> 
> >
> > >
> > > 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
> > > >