Netdev List
 help / color / mirror / Atom feed
* [PATCH net] ipmr: clear skb->cb[] in reg_vif_xmit()
@ 2026-10-25 11:13 Eric Dumazet
  2026-10-25 11:28 ` Ido Schimmel
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-10-25 11:13 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Willem de Bruijn, David Ahern, Ido Schimmel, Simon Horman, netdev,
	Eric Dumazet

reg_vif_xmit() hands the skb to ipmr_cache_report() as an
IGMPMSG_WHOLEPKT report. When the mroute socket has IP_PKTINFO
enabled, ipmr_cache_report() calls ipv4_pktinfo_prepare() on the
original skb, which reads skb->cb[] as IPCB.

By the time the pimreg device transmits the packet, skb->cb[] is not
IPCB anymore: qdisc_pkt_len_init() overwrites IPCB(skb)->iif with
qdisc_skb_cb(skb)->pkt_len, and any qdisc installed on the device or a
tc BPF program writing skb->cb[] fills the area holding the IP options
and IPCB(skb)->flags.

For locally generated packets routed out pimreg (they carry an rtable
dst), the mroute socket can therefore receive a bogus ipi_ifindex
(the packet length), and the l3mdev slave test in
ipv4_pktinfo_prepare() is driven by stale qdisc data.

Clear IPCB before building the report, as done for IPv6 in
commit 238164201431 ("ip6mr: clear skb->cb[] in reg_vif_xmit()").
The skb is freed right after, so nothing else depends on its control
block.

Fixes: bb7403655b3c ("ipmr: support IP_PKTINFO on cache report IGMP msg")
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
Issue has been discovered while reviewing Ido's series:
 https://lore.kernel.org/netdev/CAL4WiiovWOOQ9PBPbnPk2=rK+V5WgMpOFh814-D6_A-k5ope_A@mail.gmail.com/
No repro, but this is mirroring the fix we did in 238164201431 ("ip6mr: clear skb->cb[] in reg_vif_xmit()").

 net/ipv4/ipmr.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index acacaae05cc49dc15910f23f7539865432518587..8b84142fd7754d95aa1f2facf68ac554916efd27 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -550,6 +550,11 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
 	DEV_STATS_ADD(dev, tx_bytes, skb->len);
 	DEV_STATS_INC(dev, tx_packets);
 
+	/* skb->cb[] holds qdisc/tc state at this point, not IPCB.
+	 * ipmr_cache_report() reads it as IPCB to build IP_PKTINFO.
+	 */
+	memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
+
 	/* Pairs with WRITE_ONCE() in vif_add() and vif_delete() */
 	ipmr_cache_report(mrt, skb, READ_ONCE(mrt->mroute_reg_vif_num),
 			  IGMPMSG_WHOLEPKT);
-- 
2.53.0


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

* Re: [PATCH net] ipmr: clear skb->cb[] in reg_vif_xmit()
  2026-10-25 11:13 [PATCH net] ipmr: clear skb->cb[] in reg_vif_xmit() Eric Dumazet
@ 2026-10-25 11:28 ` Ido Schimmel
  2026-10-26  6:03 ` Hangbin Liu
  2026-10-26 23:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Ido Schimmel @ 2026-10-25 11:28 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	David Ahern, Simon Horman, netdev

On Sun, Oct 25, 2026 at 12:13:37PM +0100, Eric Dumazet wrote:
> reg_vif_xmit() hands the skb to ipmr_cache_report() as an
> IGMPMSG_WHOLEPKT report. When the mroute socket has IP_PKTINFO
> enabled, ipmr_cache_report() calls ipv4_pktinfo_prepare() on the
> original skb, which reads skb->cb[] as IPCB.
> 
> By the time the pimreg device transmits the packet, skb->cb[] is not
> IPCB anymore: qdisc_pkt_len_init() overwrites IPCB(skb)->iif with
> qdisc_skb_cb(skb)->pkt_len, and any qdisc installed on the device or a
> tc BPF program writing skb->cb[] fills the area holding the IP options
> and IPCB(skb)->flags.
> 
> For locally generated packets routed out pimreg (they carry an rtable
> dst), the mroute socket can therefore receive a bogus ipi_ifindex
> (the packet length), and the l3mdev slave test in
> ipv4_pktinfo_prepare() is driven by stale qdisc data.
> 
> Clear IPCB before building the report, as done for IPv6 in
> commit 238164201431 ("ip6mr: clear skb->cb[] in reg_vif_xmit()").
> The skb is freed right after, so nothing else depends on its control
> block.
> 
> Fixes: bb7403655b3c ("ipmr: support IP_PKTINFO on cache report IGMP msg")
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

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

* Re: [PATCH net] ipmr: clear skb->cb[] in reg_vif_xmit()
  2026-10-25 11:13 [PATCH net] ipmr: clear skb->cb[] in reg_vif_xmit() Eric Dumazet
  2026-10-25 11:28 ` Ido Schimmel
@ 2026-10-26  6:03 ` Hangbin Liu
  2026-10-26 23:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Hangbin Liu @ 2026-10-26  6:03 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	David Ahern, Ido Schimmel, Simon Horman, netdev

On Sun, Oct 25, 2026 at 12:13:37PM +0100, Eric Dumazet wrote:
> reg_vif_xmit() hands the skb to ipmr_cache_report() as an
> IGMPMSG_WHOLEPKT report. When the mroute socket has IP_PKTINFO
> enabled, ipmr_cache_report() calls ipv4_pktinfo_prepare() on the
> original skb, which reads skb->cb[] as IPCB.
> 
> By the time the pimreg device transmits the packet, skb->cb[] is not
> IPCB anymore: qdisc_pkt_len_init() overwrites IPCB(skb)->iif with
> qdisc_skb_cb(skb)->pkt_len, and any qdisc installed on the device or a
> tc BPF program writing skb->cb[] fills the area holding the IP options
> and IPCB(skb)->flags.
> 
> For locally generated packets routed out pimreg (they carry an rtable
> dst), the mroute socket can therefore receive a bogus ipi_ifindex
> (the packet length), and the l3mdev slave test in
> ipv4_pktinfo_prepare() is driven by stale qdisc data.
> 
> Clear IPCB before building the report, as done for IPv6 in
> commit 238164201431 ("ip6mr: clear skb->cb[] in reg_vif_xmit()").
> The skb is freed right after, so nothing else depends on its control
> block.
> 
> Fixes: bb7403655b3c ("ipmr: support IP_PKTINFO on cache report IGMP msg")
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
> Issue has been discovered while reviewing Ido's series:
>  https://lore.kernel.org/netdev/CAL4WiiovWOOQ9PBPbnPk2=rK+V5WgMpOFh814-D6_A-k5ope_A@mail.gmail.com/
> No repro, but this is mirroring the fix we did in 238164201431 ("ip6mr: clear skb->cb[] in reg_vif_xmit()").
> 
>  net/ipv4/ipmr.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index acacaae05cc49dc15910f23f7539865432518587..8b84142fd7754d95aa1f2facf68ac554916efd27 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -550,6 +550,11 @@ static netdev_tx_t reg_vif_xmit(struct sk_buff *skb, struct net_device *dev)
>  	DEV_STATS_ADD(dev, tx_bytes, skb->len);
>  	DEV_STATS_INC(dev, tx_packets);
>  
> +	/* skb->cb[] holds qdisc/tc state at this point, not IPCB.
> +	 * ipmr_cache_report() reads it as IPCB to build IP_PKTINFO.
> +	 */
> +	memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
> +
>  	/* Pairs with WRITE_ONCE() in vif_add() and vif_delete() */
>  	ipmr_cache_report(mrt, skb, READ_ONCE(mrt->mroute_reg_vif_num),
>  			  IGMPMSG_WHOLEPKT);
> -- 
> 2.53.0
> 

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>

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

* Re: [PATCH net] ipmr: clear skb->cb[] in reg_vif_xmit()
  2026-10-25 11:13 [PATCH net] ipmr: clear skb->cb[] in reg_vif_xmit() Eric Dumazet
  2026-10-25 11:28 ` Ido Schimmel
  2026-10-26  6:03 ` Hangbin Liu
@ 2026-10-26 23:50 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-10-26 23:50 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, kuba, pabeni, willemb, dsahern, idosch, horms, netdev

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sun, 25 Oct 2026 12:13:37 +0100 you wrote:
> reg_vif_xmit() hands the skb to ipmr_cache_report() as an
> IGMPMSG_WHOLEPKT report. When the mroute socket has IP_PKTINFO
> enabled, ipmr_cache_report() calls ipv4_pktinfo_prepare() on the
> original skb, which reads skb->cb[] as IPCB.
> 
> By the time the pimreg device transmits the packet, skb->cb[] is not
> IPCB anymore: qdisc_pkt_len_init() overwrites IPCB(skb)->iif with
> qdisc_skb_cb(skb)->pkt_len, and any qdisc installed on the device or a
> tc BPF program writing skb->cb[] fills the area holding the IP options
> and IPCB(skb)->flags.
> 
> [...]

Here is the summary with links:
  - [net] ipmr: clear skb->cb[] in reg_vif_xmit()
    https://git.kernel.org/netdev/net/c/389952e8776a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-10-26 23:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-25 11:13 [PATCH net] ipmr: clear skb->cb[] in reg_vif_xmit() Eric Dumazet
2026-10-25 11:28 ` Ido Schimmel
2026-10-26  6:03 ` Hangbin Liu
2026-10-26 23:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox