From: Ido Schimmel <idosch@nvidia.com>
To: Ren Wei <enjou1224z@gmail.com>
Cc: netdev@vger.kernel.org, dsahern@kernel.org, davem@davemloft.net,
edumazet@google.com, pabeni@redhat.com, horms@kernel.org,
leone4fernando@gmail.com, vega@nebusec.ai, zhilinz@nebusec.ai
Subject: Re: [PATCH net 1/1] ipmr: only copy pktinfo to cache reports
Date: Wed, 29 Jul 2026 12:12:27 +0300 [thread overview]
Message-ID: <20260729091227.GA1258429@shredder> (raw)
In-Reply-To: <5bc7cd71c2d671b305f25497d88ae8a0aa663c08.1784894076.git.zhilinz@nebusec.ai>
On Tue, Jul 28, 2026 at 12:44:49AM +0800, Ren Wei wrote:
> From: Zhiling Zou <zhilinz@nebusec.ai>
>
> ipmr_cache_report() builds short IGMP reports for mrouted from a packet
> that may be a synthetic RTM_GETROUTE query. That query skb stores the
> netlink requester portid in NETLINK_CB(), but the report is delivered to
> a raw IPv4 socket, whose receive path interprets skb->cb as IPCB().
>
> Commit bb7403655b3c ("ipmr: support IP_PKTINFO on cache report IGMP
> msg") added IP_PKTINFO support by calling ipv4_pktinfo_prepare() on the
> original packet and then copying the whole control buffer to the report
> skb. For synthetic route-query packets, this copies NETLINK_CB bytes
It's copying 48 bytes, which is the size of the control block. Please reword.
> into IPCB and lets a controlled portid corrupt IPCB(skb)->opt. With
> IP_RECVOPTS or IP_RETOPTS enabled, the raw socket receive path can then
> copy past the short report packet or overflow the stack option buffer.
>
> Keep the IP_PKTINFO support, but initialize the report skb control buffer
> and copy only the pktinfo fields prepared by ipv4_pktinfo_prepare().
This can break IP_RECVOPTS and IP_RETOPTS on the mrouted socket, but
this was never meant to work (side effect of bb7403655b3c) and therefore
unlikely to cause any regressions. Please mention this in the commit
message.
>
> Fixes: bb7403655b3c ("ipmr: support IP_PKTINFO on cache report IGMP msg")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Assisted-by: Codex:gpt-5.6-terra
> Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
> Signed-off-by: Ren Wei <enjou1224z@gmail.com>
> ---
> net/ipv4/ipmr.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 1d9a4ac14fcef..783de5de02ddd 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -1061,6 +1061,7 @@ static int ipmr_cache_report(const struct mr_table *mrt,
> + struct in_pktinfo *info;
> struct sock *mroute_sk;
> struct igmphdr *igmp;
> struct igmpmsg *msg;
> struct sk_buff *skb;
> int ret;
>
> @@ -1113,7 +1114,10 @@ static int ipmr_cache_report(const struct mr_table *mrt,
> 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));
> + info = PKTINFO_SKB_CB(skb);
> + memset(skb->cb, 0, sizeof(skb->cb));
> + info->ipi_ifindex = PKTINFO_SKB_CB(pkt)->ipi_ifindex;
> + info->ipi_spec_dst = PKTINFO_SKB_CB(pkt)->ipi_spec_dst;
> /* Add our header.
> * Note that code, csum and group fields are cleared.
> */
I believe that Sashiko is correct and there is a similar problem with
the other branch:
https://sashiko.dev/#/patchset/5bc7cd71c2d671b305f25497d88ae8a0aa663c08.1784894076.git.zhilinz%40nebusec.ai
This should fix both:
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 1d9a4ac14fce..4827973c6f9c 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1058,6 +1058,7 @@ static int ipmr_cache_report(const struct mr_table *mrt,
struct sk_buff *pkt, vifi_t vifi, int assert)
{
const int ihl = ip_hdrlen(pkt);
+ struct in_pktinfo *info;
struct sock *mroute_sk;
struct igmphdr *igmp;
struct igmpmsg *msg;
@@ -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;
}
+ ipv4_pktinfo_prepare(mroute_sk, pkt, false);
+ memset(skb->cb, 0, sizeof(skb->cb));
+ info = PKTINFO_SKB_CB(skb);
+ info->ipi_ifindex = PKTINFO_SKB_CB(pkt)->ipi_ifindex;
+ info->ipi_spec_dst = PKTINFO_SKB_CB(pkt)->ipi_spec_dst;
+
igmpmsg_netlink_event(mrt, skb);
/* Deliver to mrouted */
next prev parent reply other threads:[~2026-07-29 9:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-27 16:44 [PATCH net 0/1] ipmr: only copy pktinfo to cache reports Ren Wei
2026-07-27 16:44 ` [PATCH net 1/1] " Ren Wei
2026-07-29 9:12 ` Ido Schimmel [this message]
2026-07-29 14:46 ` zhilin zou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260729091227.GA1258429@shredder \
--to=idosch@nvidia.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=enjou1224z@gmail.com \
--cc=horms@kernel.org \
--cc=leone4fernando@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=vega@nebusec.ai \
--cc=zhilinz@nebusec.ai \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.