Re: [PATCH net v3 1/1] ipmr: only copy pktinfo to cache reports

Ido Schimmel <[email protected]>
Newsgroups org.kernel.vger.netdev
Message-ID <20260805081737.GA1284302@shredder>
On Tue, Aug 04, 2026 at 06:31:26PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "Looks legit, let's fix both in one patch?"

I suggested [1], but the bug in the IGMPMSG_WHOLEPKT /
IGMPMSG_WRVIFWHOLE branch needs a different Fixes tag (it precedes
bb7403655b3c) and Sashiko [2][3] found another bug in this code:

ipv4_pktinfo_prepare() modifies the control block of 'pkt', which can
result in this packet being processed incorrectly after
ipmr_cache_report() returns. I *think* that [4] takes care of it, but
all of this needs to be explained in the commit message which should
carry the appropriate Fixes tags (bb7403655b3c + 1da177e4c3f4).

Note that this upcall is not fast path, so the extra work shouldn't be a
problem.

Is this what you prefer?

[1] https://lore.kernel.org/all/20260729091227.GA1258429@shredder/
[2] https://sashiko.dev/#/patchset/1b809975e5bd9c0a1dd6fdd1db534e701fe5a4b6.1785379072.git.zhilinz%40nebusec.ai
[3] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/1b809975e5bd9c0a1dd6fdd1db534e701fe5a4b6.1785379072.git.zhilinz%40nebusec.ai
[4]
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 1d9a4ac14fce..cdc84eb05c79 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1057,6 +1057,7 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
 static int ipmr_cache_report(const struct mr_table *mrt,
 			     struct sk_buff *pkt, vifi_t vifi, int assert)
 {
+	unsigned char pkt_cb[sizeof(pkt->cb)];
 	const int ihl = ip_hdrlen(pkt);
 	struct sock *mroute_sk;
 	struct igmphdr *igmp;
@@ -1112,8 +1113,6 @@ static int ipmr_cache_report(const struct mr_table *mrt,
 		msg = (struct igmpmsg *)skb_network_header(skb);
 		msg->im_vif = vifi;
 		msg->im_vif_hi = vifi >> 8;
-		ipv4_pktinfo_prepare(mroute_sk, pkt, false);
-		memcpy(skb->cb, pkt->cb, sizeof(skb->cb));
 		/* Add our header.
 		 * Note that code, csum and group fields are cleared.
 		 */
@@ -1124,6 +1123,12 @@ static int ipmr_cache_report(const struct mr_table *mrt,
 		skb->transport_header = skb->network_header;
 	}
 
+	memcpy(pkt_cb, pkt->cb, sizeof(pkt_cb));
+	ipv4_pktinfo_prepare(mroute_sk, pkt, false);
+	memset(skb->cb, 0, sizeof(skb->cb));
+	*PKTINFO_SKB_CB(skb) = *PKTINFO_SKB_CB(pkt);
+	memcpy(pkt->cb, pkt_cb, sizeof(pkt_cb));
+
 	igmpmsg_netlink_event(mrt, skb);
 
 	/* Deliver to mrouted */
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.