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

[luoxuanqiang]

在 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?

> > Signed-off-by: Xuanqiang Luo<luoxuanqiang@xxxxxxxxxx>
> > ---
> > v2:
> > - Use dst_dev_rcu() and dev_net_rcu() for the RCU readers.
> > - Use rcu_assign_pointer() when publishing the uncached route device
> >    replacement.
> > - Slightly adjust the commit message wording because this issue was found
> >    by inspection, not from an observed user-visible failure.
> >
> > v1:https://lore.kernel.org/all/20260630094250.29386-1-xuanqiang.luo@xxxxxxxxx/
> >
> >   net/ipv4/route.c | 29 +++++++++++++++++------------
> >   1 file changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index 3f3de5164d6e5..57f38467e6d0c 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -873,6 +873,7 @@ static void ipv4_negative_advice(struct sock *sk,
> >   void ip_rt_send_redirect(struct sk_buff *skb)
> >   {
> >   	struct rtable *rt = skb_rtable(skb);
> > +	struct net_device *dev;
> https://docs.kernel.org/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs
>
> Same in other places.

Thanks for pointing this out!
I will fix the local variable ordering in the next version.