[PATCH net v2 1/1] ipmr: validate VIF indexes before offload lookup

A packet can race with VIF publication and make ipmr_find_vif() return
-1 even though the VIF device pointer is already visible. In the IPv4
path, checking only the MFC parent device then allows forwarding to
continue with an invalid ingress VIF. Besides the offload lookup, this
can let (*,G) forwarding loop the packet back out the ingress VIF.

Reject packets whose ingress VIF cannot be found in both IPv4 and IPv6
forwarding. The IPv4 offload helper also checks for a negative ingress
index before deriving vif_table pointers. The output index comes from
the bounded MFC output-interface iteration, so it does not need a
separate check.

The out-of-bounds read in ipmr_forward_offloaded() requires an skb
marked for L3 forwarding offload, which is currently done by mlxsw.
The race is rare; the supplied PoC triggers a UBSAN report for the
negative array index, but does not demonstrate an actual out-of-bounds
read.

Fixes: 4eadb88244d1 ("ipmr: do not acquire mrt_lock while calling ip_mr_forward()")
Fixes: 9b1c21d898fd ("ip6mr: do not acquire mrt_lock while calling ip6_mr_forward()")
Cc: stable@vger.kernel.org
Reported-by: Vega <redacted>
Link: https://lore.kernel.org/netdev/b966eb102e98f299e45be59ada033a8766e4c6df.1791899747.git.zhilinz@nebusec.ai/ (local)
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Suggested-by: Naman Gulati <redacted>
Assisted-by: LLM
Signed-off-by: Zhiling Zou <redacted>
---
changes in v2:
  - Reject packets with an unknown ingress VIF in IPv4 and IPv6 forwarding.
  - Keep only the negative in_vifi check in the offload helper; out_vifi is
    bounded by the MFC output-interface iteration.
  - Add the Fixes tag for the corresponding IPv6 forwarding change.
  - Clarify that an actual OOB read requires the mlxsw offload mark and that
    the PoC reports the UBSAN negative-index diagnostic only.
  - v1 Link: https://lore.kernel.org/netdev/b966eb102e98f299e45be59ada033a8766e4c6df.1791899747.git.zhilinz@nebusec.ai/ (local)

 net/ipv4/ipmr.c  | 11 ++++++++---
 net/ipv6/ip6mr.c |  3 ++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index b9c544d48c452..d10a66534ef9f 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c

@@ -1879,11 +1879,15 @@ static inline int ipmr_forward_finish(struct net *net, struct sock *sk,
 static bool ipmr_forward_offloaded(struct sk_buff *skb, struct mr_table *mrt,
 				   int in_vifi, int out_vifi)
 {
-	struct vif_device *out_vif = &mrt->vif_table[out_vifi];
-	struct vif_device *in_vif = &mrt->vif_table[in_vifi];
+	struct vif_device *out_vif, *in_vif;
 
 	if (!skb->offload_l3_fwd_mark)
 		return false;
+	if (in_vifi < 0)
+		return false;
+
+	out_vif = &mrt->vif_table[out_vifi];
+	in_vif = &mrt->vif_table[in_vifi];
 	if (!out_vif->dev_parent_id.id_len || !in_vif->dev_parent_id.id_len)
 		return false;
 	return netdev_phys_item_id_same(&out_vif->dev_parent_id,

@@ -2064,7 +2068,8 @@ static void ip_mr_forward(struct net *net, struct mr_table *mrt,
 	}
 
 	/* Wrong interface: drop packet and (maybe) send PIM assert. */
-	if (rcu_access_pointer(mrt->vif_table[vif].dev) != dev) {
+	if (true_vifi < 0 ||
+	    rcu_access_pointer(mrt->vif_table[vif].dev) != dev) {
 		if (rt_is_output_route(skb_rtable(skb))) {
 			/* It is our own packet, looped back.
 			 * Very complicated situation...

diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 9d8116b5edb17..faaf04f5c4f33 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c

@@ -2218,7 +2218,8 @@ static void ip6_mr_forward(struct net *net, struct mr_table *mrt,
 	/*
 	 * Wrong interface: drop packet and (maybe) send PIM assert.
 	 */
-	if (rcu_access_pointer(mrt->vif_table[vif].dev) != dev) {
+	if (true_vifi < 0 ||
+	    rcu_access_pointer(mrt->vif_table[vif].dev) != dev) {
 		atomic_long_inc(&c->_c.mfc_un.res.wrong_if);
 
 		if (true_vifi >= 0 && READ_ONCE(mrt->mroute_do_assert) &&

-- 
2.55.0