Re: [PATCH net-next v2] ipv4: hold a consistent view of rt->dst.dev under RCU

[luoxuanqiang]

在 2026/7/7 17:02, Paolo Abeni 写道:
> On 7/7/26 3:26 AM, luoxuanqiang wrote:
> > 在 2026/7/6 00:46, Ido Schimmel 写道:
> > > On Wed, Jul 01, 2026 at 11:24:34AM +0800,xuanqiang.luo@xxxxxxxxx wrote:
> > > > From: Xuanqiang Luo<luoxuanqiang@xxxxxxxxxx>
> > > >
> > > > rt_flush_dev() walks the per-CPU uncached route list and rewrites
> > > > rt->dst.dev in-place to blackhole_netdev under spin_lock_bh().
> > > > This lock does not exclude RCU readers, which may load rt->dst.dev
> > > > multiple times within a single rcu_read_lock() region.
> > > >
> > > > ip_rt_send_redirect() is a typical example: it reads rt->dst.dev
> > > > three times to obtain in_dev, the L3 master ifindex, and net.
> > > > A concurrent device unregistration can repoint rt->dst.dev to
> > > > blackhole_netdev between those reads, making the reader combine
> > > > state from two different net_devices — for instance, an in_dev
> > > > from the real device but a netns and peer lookup from the blackhole
> > > > device.  ip_rt_get_source() has the same problem: it reads
> > > > rt->dst.dev four times to obtain the output ifindex, the netns,
> > > > and the source address, so a concurrent flush can cause the source
> > > > selection to mix state from different devices.
> > > Why only change ip_rt_send_redirect() and ip_rt_get_source() when the
> > > patch is titled "ipv4: hold a consistent view of rt->dst.dev under RCU"?
> > > What is the criterion?
> > Thanks! You are right, the subject is too broad. I will make them
> > more accurate in the next version.
> >
> > > > Take a single dst_dev_rcu() snapshot of rt->dst.dev at the start
> > > > of each affected RCU reader and use that snapshot throughout, so
> > > > concurrent flushes cannot cause mid-function inconsistency.
> > > > Publish the in-place write in rt_flush_dev() with rcu_assign_pointer()
> > > > to match the readers.
> > > The rt_flush_dev() change should be a separate change. Note that
> > > dst_dev_put() was already converted to use rcu_assign_pointer().
> > I will split the rt_flush_dev() change into a separate patch.
> >
> > > > Fixes: caacf05e5ad1a ("ipv4: Properly purge netdev references on uncached routes.")
> > > Please remove the Fixes tag given you are targeting net-next.
> > Just to clarify: is the suggestion to drop the Fixes tag here solely
> > because this patch is targeted at net-next? Or are there any other
> > reasons?
> Generally speaking, yes: net-next patches should not include a fixes tag
> unless the blamed commit is on net-next only.
>
> More specifically, this patch is really a behavior improvement and not a
> vertical fix, as such we want to avoid it propagating on stable trees,
> as the fixes tag sometimes does.
>
> /P
Many thanks for the detailed explanation!