linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 1/2] vrf: Drop existing dst reference in vrf_ip6_input_dst
@ 2025-07-22 21:02 Stanislav Fomichev
  2025-07-22 21:02 ` [PATCH net 2/2] net: Warn when overriding referenced dst entry Stanislav Fomichev
  2025-07-23  9:09 ` [syzbot ci] Re: vrf: Drop existing dst reference in vrf_ip6_input_dst syzbot ci
  0 siblings, 2 replies; 7+ messages in thread
From: Stanislav Fomichev @ 2025-07-22 21:02 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, dsahern, andrew+netdev, horms,
	linux-kernel

Commit 8fba0fc7b3de ("selftests: tc: Add generic erspan_opts matching support
for tc-flower") started triggering the following kmemleak warning:

unreferenced object 0xffff888015fb0e00 (size 512):
  comm "softirq", pid 0, jiffies 4294679065
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 40 d2 85 9e ff ff ff ff  ........@.......
    41 69 59 9d ff ff ff ff 00 00 00 00 00 00 00 00  AiY.............
  backtrace (crc 30b71e8b):
    __kmalloc_noprof+0x359/0x460
    metadata_dst_alloc+0x28/0x490
    erspan_rcv+0x4f1/0x1160 [ip_gre]
    gre_rcv+0x217/0x240 [ip_gre]
    gre_rcv+0x1b8/0x400 [gre]
    ip_protocol_deliver_rcu+0x31d/0x3a0
    ip_local_deliver_finish+0x37d/0x620
    ip_local_deliver+0x174/0x460
    ip_rcv+0x52b/0x6b0
    __netif_receive_skb_one_core+0x149/0x1a0
    process_backlog+0x3c8/0x1390
    __napi_poll.constprop.0+0xa1/0x390
    net_rx_action+0x59b/0xe00
    handle_softirqs+0x22b/0x630
    do_softirq+0xb1/0xf0
    __local_bh_enable_ip+0x115/0x150

vrf_ip6_input_dst unconditionally sets skb dst entry, add a call to
skb_dst_drop to drop any existing entry.

Cc: David Ahern <dsahern@kernel.org>
Fixes: 9ff74384600a ("net: vrf: Handle ipv6 multicast and link-local addresses")
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 drivers/net/vrf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 9a4beea6ee0c..3ccd649913b5 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1302,6 +1302,8 @@ static void vrf_ip6_input_dst(struct sk_buff *skb, struct net_device *vrf_dev,
 	struct net *net = dev_net(vrf_dev);
 	struct rt6_info *rt6;
 
+	skb_dst_drop(skb);
+
 	rt6 = vrf_ip6_route_lookup(net, vrf_dev, &fl6, ifindex, skb,
 				   RT6_LOOKUP_F_HAS_SADDR | RT6_LOOKUP_F_IFACE);
 	if (unlikely(!rt6))
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net 2/2] net: Warn when overriding referenced dst entry
  2025-07-22 21:02 [PATCH net 1/2] vrf: Drop existing dst reference in vrf_ip6_input_dst Stanislav Fomichev
@ 2025-07-22 21:02 ` Stanislav Fomichev
  2025-07-23  2:00   ` Jakub Kicinski
  2025-07-23  9:09 ` [syzbot ci] Re: vrf: Drop existing dst reference in vrf_ip6_input_dst syzbot ci
  1 sibling, 1 reply; 7+ messages in thread
From: Stanislav Fomichev @ 2025-07-22 21:02 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, dsahern, andrew+netdev, horms,
	linux-kernel

Add WARN_ON_ONCE with CONFIG_NET_DEBUG to warn on overriding referenced
dst entries. This should get a better signal than tracing leaked
objects from kmemleak.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 include/linux/skbuff.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5520524c93bf..c89931ac01e5 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1159,6 +1159,12 @@ static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
 	return (struct dst_entry *)(skb->_skb_refdst & SKB_DST_PTRMASK);
 }
 
+static inline void skb_dst_check(struct sk_buff *skb)
+{
+	DEBUG_NET_WARN_ON_ONCE(skb_dst(skb) &&
+			       !(skb->_skb_refdst & SKB_DST_NOREF));
+}
+
 /**
  * skb_dst_set - sets skb dst
  * @skb: buffer
@@ -1169,6 +1175,7 @@ static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
  */
 static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
 {
+	skb_dst_check(skb);
 	skb->slow_gro |= !!dst;
 	skb->_skb_refdst = (unsigned long)dst;
 }
@@ -1185,6 +1192,7 @@ static inline void skb_dst_set(struct sk_buff *skb, struct dst_entry *dst)
  */
 static inline void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
 {
+	skb_dst_check(skb);
 	WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
 	skb->slow_gro |= !!dst;
 	skb->_skb_refdst = (unsigned long)dst | SKB_DST_NOREF;
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net 2/2] net: Warn when overriding referenced dst entry
  2025-07-22 21:02 ` [PATCH net 2/2] net: Warn when overriding referenced dst entry Stanislav Fomichev
@ 2025-07-23  2:00   ` Jakub Kicinski
  2025-07-23 15:13     ` Stanislav Fomichev
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2025-07-23  2:00 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: netdev, davem, edumazet, pabeni, dsahern, andrew+netdev, horms,
	linux-kernel

On Tue, 22 Jul 2025 14:02:56 -0700 Stanislav Fomichev wrote:
> Add WARN_ON_ONCE with CONFIG_NET_DEBUG to warn on overriding referenced
> dst entries. This should get a better signal than tracing leaked
> objects from kmemleak.

Looks like we're tripping the severe nastiness in icmp.c:

		/* Ugh! */
		orefdst = skb_in->_skb_refdst; /* save old refdst */
		skb_dst_set(skb_in, NULL);
		err = ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr,
				     dscp, rt2->dst.dev) ? -EINVAL : 0;

		dst_release(&rt2->dst);
		rt2 = skb_rtable(skb_in);
		skb_in->_skb_refdst = orefdst; /* restore old refdst */

There's more of these around. I think we need a new helper for
"saving" and "restoring" the refdst? Either way, I reckon the
patch with the check belongs in net-next.

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 5520524c93bf..c89931ac01e5 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1159,6 +1159,12 @@ static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
>  	return (struct dst_entry *)(skb->_skb_refdst & SKB_DST_PTRMASK);
>  }
>  
> +static inline void skb_dst_check(struct sk_buff *skb)

skb_dst_check_unset() ? Right now we assume the caller will override.
Or will there be more conditions added?

> +{
> +	DEBUG_NET_WARN_ON_ONCE(skb_dst(skb) &&
> +			       !(skb->_skb_refdst & SKB_DST_NOREF));

Why not 

	DEBUG_NET_WARN_ON_ONCE(skb->_skb_refdst & SKB_DST_PTRMASK);

?
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [syzbot ci] Re: vrf: Drop existing dst reference in vrf_ip6_input_dst
  2025-07-22 21:02 [PATCH net 1/2] vrf: Drop existing dst reference in vrf_ip6_input_dst Stanislav Fomichev
  2025-07-22 21:02 ` [PATCH net 2/2] net: Warn when overriding referenced dst entry Stanislav Fomichev
@ 2025-07-23  9:09 ` syzbot ci
  1 sibling, 0 replies; 7+ messages in thread
From: syzbot ci @ 2025-07-23  9:09 UTC (permalink / raw)
  To: andrew, davem, dsahern, edumazet, horms, kuba, linux-kernel,
	netdev, pabeni, sdf
  Cc: syzbot

syzbot ci has tested the following series

[v1] vrf: Drop existing dst reference in vrf_ip6_input_dst
https://lore.kernel.org/all/<20250722210256.143208-1-sdf@fomichev.me>
* [PATCH net 1/2] vrf: Drop existing dst reference in vrf_ip6_input_dst
* [PATCH net 2/2] net: Warn when overriding referenced dst entry

and found the following issue:
WARNING in nf_reject_fill_skb_dst

Full report is available here:
https://ci.syzbot.org/series/c92e1af5-3b8b-4c26-8104-acca93ad8939

***

WARNING in nf_reject_fill_skb_dst

tree:      net
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/netdev/net.git
base:      cf074eca0065bc5142e6004ae236bb35a2687fdf
arch:      amd64
compiler:  Debian clang version 20.1.7 (++20250616065708+6146a88f6049-1~exp1~20250616065826.132), Debian LLD 20.1.7
config:    https://ci.syzbot.org/builds/235c4af3-77ea-4670-9c9e-9de1ef0a5aa6/config
syz repro: https://ci.syzbot.org/findings/1038832f-72d8-4491-9ac9-28339d31f0f7/syz_repro

------------[ cut here ]------------
WARNING: CPU: 1 PID: 12 at ./include/linux/skbuff.h:1165 nf_reject_fill_skb_dst+0x313/0x3b0
Modules linked in:
CPU: 1 UID: 0 PID: 12 Comm: kworker/u8:0 Not tainted 6.16.0-rc6-syzkaller-00135-gcf074eca0065-dirty #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
Workqueue: ipv6_addrconf addrconf_dad_work
RIP: 0010:nf_reject_fill_skb_dst+0x313/0x3b0
Code: 8b 0d f1 35 98 08 48 3b 8c 24 e0 00 00 00 75 6b 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f 5d e9 54 19 69 01 cc e8 9e 7e b8 f7 90 <0f> 0b 90 e9 43 ff ff ff e8 90 7e b8 f7 90 0f 0b 90 e9 e3 fe ff ff
RSP: 0018:ffffc900001e0360 EFLAGS: 00010246
RAX: ffffffff8a07aa52 RBX: ffff888110f75500 RCX: ffff88801c2f5640
RDX: 0000000000000100 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffc900001e0490 R08: ffffffff8fa1d6f7 R09: 1ffffffff1f43ade
R10: dffffc0000000000 R11: fffffbfff1f43adf R12: ffff888110f75501
R13: dffffc0000000001 R14: 1ffff9200003c070 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff8881a3c22000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00002000000000c0 CR3: 000000001ff6e000 CR4: 00000000000006f0
Call Trace:
 <IRQ>
 nf_send_unreach+0x17b/0x700
 nft_reject_inet_eval+0x4bc/0x690
 nft_do_chain+0x40c/0x1920
 nft_do_chain_inet+0x25d/0x340
 nf_hook_slow+0xc5/0x220
 NF_HOOK+0x206/0x3a0
 __netif_receive_skb+0x143/0x380
 process_backlog+0x60e/0x14f0
 __napi_poll+0xc7/0x480
 net_rx_action+0x707/0xe30
 handle_softirqs+0x286/0x870
 do_softirq+0xec/0x180
 </IRQ>
 <TASK>
 __local_bh_enable_ip+0x17d/0x1c0
 __dev_queue_xmit+0x1cd7/0x3a70
 ip6_finish_output2+0x11fe/0x16a0
 ndisc_send_skb+0xc77/0x1500
 ndisc_send_ns+0xcb/0x150
 addrconf_dad_work+0xaae/0x14b0
 process_scheduled_works+0xae1/0x17b0
 worker_thread+0x8a0/0xda0
 kthread+0x711/0x8a0
 ret_from_fork+0x3fc/0x770
 ret_from_fork_asm+0x1a/0x30
 </TASK>


---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 2/2] net: Warn when overriding referenced dst entry
  2025-07-23  2:00   ` Jakub Kicinski
@ 2025-07-23 15:13     ` Stanislav Fomichev
  2025-07-23 15:24       ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Stanislav Fomichev @ 2025-07-23 15:13 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Stanislav Fomichev, netdev, davem, edumazet, pabeni, dsahern,
	andrew+netdev, horms, linux-kernel

On 07/22, Jakub Kicinski wrote:
> On Tue, 22 Jul 2025 14:02:56 -0700 Stanislav Fomichev wrote:
> > Add WARN_ON_ONCE with CONFIG_NET_DEBUG to warn on overriding referenced
> > dst entries. This should get a better signal than tracing leaked
> > objects from kmemleak.
> 
> Looks like we're tripping the severe nastiness in icmp.c:
> 
> 		/* Ugh! */
> 		orefdst = skb_in->_skb_refdst; /* save old refdst */
> 		skb_dst_set(skb_in, NULL);
> 		err = ip_route_input(skb_in, fl4_dec.daddr, fl4_dec.saddr,
> 				     dscp, rt2->dst.dev) ? -EINVAL : 0;
> 
> 		dst_release(&rt2->dst);
> 		rt2 = skb_rtable(skb_in);
> 		skb_in->_skb_refdst = orefdst; /* restore old refdst */
> 
> There's more of these around. I think we need a new helper for
> "saving" and "restoring" the refdst? Either way, I reckon the
> patch with the check belongs in net-next.

SG, will run all tests locally and try to weed these out before
reposting for net-next.

> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 5520524c93bf..c89931ac01e5 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -1159,6 +1159,12 @@ static inline struct dst_entry *skb_dst(const struct sk_buff *skb)
> >  	return (struct dst_entry *)(skb->_skb_refdst & SKB_DST_PTRMASK);
> >  }
> >  
> > +static inline void skb_dst_check(struct sk_buff *skb)
> 
> skb_dst_check_unset() ? Right now we assume the caller will override.
> Or will there be more conditions added?

skb_dst_check_unset sounds good. I was not planning to add more checks.

> > +{
> > +	DEBUG_NET_WARN_ON_ONCE(skb_dst(skb) &&
> > +			       !(skb->_skb_refdst & SKB_DST_NOREF));
> 
> Why not 
> 
> 	DEBUG_NET_WARN_ON_ONCE(skb->_skb_refdst & SKB_DST_PTRMASK);
> 
> ?

That's more precise, agreed!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 2/2] net: Warn when overriding referenced dst entry
  2025-07-23 15:13     ` Stanislav Fomichev
@ 2025-07-23 15:24       ` Jakub Kicinski
  2025-07-23 15:30         ` Stanislav Fomichev
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2025-07-23 15:24 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Stanislav Fomichev, netdev, davem, edumazet, pabeni, dsahern,
	andrew+netdev, horms, linux-kernel

On Wed, 23 Jul 2025 08:13:25 -0700 Stanislav Fomichev wrote:
> > > +{
> > > +	DEBUG_NET_WARN_ON_ONCE(skb_dst(skb) &&
> > > +			       !(skb->_skb_refdst & SKB_DST_NOREF));  
> > 
> > Why not 
> > 
> > 	DEBUG_NET_WARN_ON_ONCE(skb->_skb_refdst & SKB_DST_PTRMASK);
> > 
> > ?  
> 
> That's more precise, agreed!

Just to be clear -- looks like I ate the

  !(skb->_skb_refdst & SKB_DST_NOREF)

part part of the condition. I think we still want that.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 2/2] net: Warn when overriding referenced dst entry
  2025-07-23 15:24       ` Jakub Kicinski
@ 2025-07-23 15:30         ` Stanislav Fomichev
  0 siblings, 0 replies; 7+ messages in thread
From: Stanislav Fomichev @ 2025-07-23 15:30 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Stanislav Fomichev, netdev, davem, edumazet, pabeni, dsahern,
	andrew+netdev, horms, linux-kernel

On 07/23, Jakub Kicinski wrote:
> On Wed, 23 Jul 2025 08:13:25 -0700 Stanislav Fomichev wrote:
> > > > +{
> > > > +	DEBUG_NET_WARN_ON_ONCE(skb_dst(skb) &&
> > > > +			       !(skb->_skb_refdst & SKB_DST_NOREF));  
> > > 
> > > Why not 
> > > 
> > > 	DEBUG_NET_WARN_ON_ONCE(skb->_skb_refdst & SKB_DST_PTRMASK);
> > > 
> > > ?  
> > 
> > That's more precise, agreed!
> 
> Just to be clear -- looks like I ate the
> 
>   !(skb->_skb_refdst & SKB_DST_NOREF)
> 
> part part of the condition. I think we still want that.

Ah, so you only want to get rid of those WARN_ONs in skb_dst, makes
sense.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-23 15:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-22 21:02 [PATCH net 1/2] vrf: Drop existing dst reference in vrf_ip6_input_dst Stanislav Fomichev
2025-07-22 21:02 ` [PATCH net 2/2] net: Warn when overriding referenced dst entry Stanislav Fomichev
2025-07-23  2:00   ` Jakub Kicinski
2025-07-23 15:13     ` Stanislav Fomichev
2025-07-23 15:24       ` Jakub Kicinski
2025-07-23 15:30         ` Stanislav Fomichev
2025-07-23  9:09 ` [syzbot ci] Re: vrf: Drop existing dst reference in vrf_ip6_input_dst syzbot ci

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).