Netdev List
 help / color / mirror / Atom feed
* [PATCH net v2] net/sched: act_ife: validate metadata length before decoding
@ 2026-09-21 12:54 Fang Xieyan
  2026-09-24 13:30 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 2+ messages in thread
From: Fang Xieyan @ 2026-09-21 12:54 UTC (permalink / raw)
  To: netdev, Jamal Hadi Salim, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, Cong Wang

skbmark_decode(), skbprio_decode() and skbtcindex_decode() read fixed-size
values from the TLV payload without validating its length.

A malformed IFE frame can declare a shorter payload, causing the decoders
to consume bytes beyond the declared metadata value:

[TLV type=IFE_META_SKBMARK len=4]
-> dlen == 0, but decode reads 4 bytes

The decoder may therefore set skb metadata from unintended input.

Validate the payload length before decoding and return -EINVAL for
invalid lengths. Read the values with get_unaligned_be32() and
get_unaligned_be16(), as TLV payloads are not guaranteed to be
aligned. Teach tcf_ife_decode() to log a decoder error separately
from an unknown metaid; both are counted as overlimits and decoding
continues with the remaining metadata.

The metadata length issue was found by an automated audit of the IFE
decode path at v6.18-rc7 and reproduced with a userspace sanitizer
model of the decode path. Compile-tested on x86_64 with defconfig and
NET_ACT_IFE=y: act_ife.o and the three act_meta_*.o build
warning-free.

Fixes: 084e2f6566d2 ("Support to encoding decoding skb mark on IFE action")
Fixes: 200e10f46936 ("Support to encoding decoding skb prio on IFE action")
Fixes: 408fbc22ef1e ("net sched ife action: Introduce skb tcindex metadata encap decap")
Assisted-by: Hawkeye:GLM-5.3-flash
Assisted-by: Qoder:Qwen3.8-Max
Signed-off-by: Fang Xieyan <fangxy@xiaopeng.com>
---

v1 -> v2:
- Read metadata values with get_unaligned_be32()/get_unaligned_be16()
  to avoid misaligned loads on strict-alignment architectures
  (Sashiko review).
- Count all undecodable metadata as overlimits in one place and log
  decoder errors separately from unknown metaids, printing the errno
  so future error types need no caller change (Sashiko review).
- Move the discovery and testing description into the commit message,
  per netdev-bot guidance.

v1: https://lore.kernel.org/all/20260920074245.79965-1-fangxy@xiaopeng.com/

 net/sched/act_ife.c             | 17 ++++++++++++-----
 net/sched/act_meta_mark.c       |  6 ++++--
 net/sched/act_meta_skbprio.c    |  6 ++++--
 net/sched/act_meta_skbtcindex.c |  6 ++++--
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 9cea71fc1db3..2afd68983ece 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -737,6 +737,7 @@ static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a,
 		u8 *curr_data;
 		u16 mtype;
 		u16 dlen;
+		int ret;
 
 		curr_data = ife_tlv_meta_decode(tlv_data, ifehdr_end, &mtype,
 						&dlen, NULL);
@@ -745,13 +746,19 @@ static int tcf_ife_decode(struct sk_buff *skb, const struct tc_action *a,
 			return TC_ACT_SHOT;
 		}
 
-		if (find_decode_metaid(skb, p, mtype, dlen, curr_data)) {
-			/* abuse overlimits to count when we receive metadata
-			 * but dont have an ops for it
+		ret = find_decode_metaid(skb, p, mtype, dlen, curr_data);
+		if (ret < 0) {
+			/* abuse overlimits to count metadata we cannot
+			 * decode: no ops for it, or the decoder rejected it
 			 */
-			pr_info_ratelimited("Unknown metaid %d dlen %d\n",
-					    mtype, dlen);
 			qstats_cpu_overlimit_inc(ife->common.cpu_qstats);
+
+			if (ret == -ENOENT)
+				pr_info_ratelimited("Unknown metaid %d dlen %d\n",
+						    mtype, dlen);
+			else
+				pr_info_ratelimited("Failed to decode metaid %d dlen %d err %d\n",
+						    mtype, dlen, ret);
 		}
 	}
 
diff --git a/net/sched/act_meta_mark.c b/net/sched/act_meta_mark.c
index ea0573cb8b2d..e2f61b22bf0f 100644
--- a/net/sched/act_meta_mark.c
+++ b/net/sched/act_meta_mark.c
@@ -10,6 +10,7 @@
 #include <linux/string.h>
 #include <linux/errno.h>
 #include <linux/skbuff.h>
+#include <linux/unaligned.h>
 #include <linux/rtnetlink.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -28,9 +29,10 @@ static int skbmark_encode(struct sk_buff *skb, void *skbdata,
 
 static int skbmark_decode(struct sk_buff *skb, void *data, u16 len)
 {
-	u32 ifemark = *(u32 *)data;
+	if (len != sizeof(u32))
+		return -EINVAL;
 
-	skb->mark = ntohl(ifemark);
+	skb->mark = get_unaligned_be32(data);
 	return 0;
 }
 
diff --git a/net/sched/act_meta_skbprio.c b/net/sched/act_meta_skbprio.c
index 2df3133ce5ad..5cdb57931eab 100644
--- a/net/sched/act_meta_skbprio.c
+++ b/net/sched/act_meta_skbprio.c
@@ -10,6 +10,7 @@
 #include <linux/string.h>
 #include <linux/errno.h>
 #include <linux/skbuff.h>
+#include <linux/unaligned.h>
 #include <linux/rtnetlink.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -33,9 +34,10 @@ static int skbprio_encode(struct sk_buff *skb, void *skbdata,
 
 static int skbprio_decode(struct sk_buff *skb, void *data, u16 len)
 {
-	u32 ifeprio = *(u32 *)data;
+	if (len != sizeof(u32))
+		return -EINVAL;
 
-	skb->priority = ntohl(ifeprio);
+	skb->priority = get_unaligned_be32(data);
 	return 0;
 }
 
diff --git a/net/sched/act_meta_skbtcindex.c b/net/sched/act_meta_skbtcindex.c
index 44547caead46..8803710c0905 100644
--- a/net/sched/act_meta_skbtcindex.c
+++ b/net/sched/act_meta_skbtcindex.c
@@ -10,6 +10,7 @@
 #include <linux/string.h>
 #include <linux/errno.h>
 #include <linux/skbuff.h>
+#include <linux/unaligned.h>
 #include <linux/rtnetlink.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -28,9 +29,10 @@ static int skbtcindex_encode(struct sk_buff *skb, void *skbdata,
 
 static int skbtcindex_decode(struct sk_buff *skb, void *data, u16 len)
 {
-	u16 ifetc_index = *(u16 *)data;
+	if (len != sizeof(u16))
+		return -EINVAL;
 
-	skb->tc_index = ntohs(ifetc_index);
+	skb->tc_index = get_unaligned_be16(data);
 	return 0;
 }
 
-- 
2.50.1


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

* Re: [PATCH net v2] net/sched: act_ife: validate metadata length before decoding
  2026-09-21 12:54 [PATCH net v2] net/sched: act_ife: validate metadata length before decoding Fang Xieyan
@ 2026-09-24 13:30 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-24 13:30 UTC (permalink / raw)
  To: Fang Xieyan
  Cc: netdev, jhs, jiri, davem, edumazet, kuba, pabeni, horms,
	xiyou.wangcong

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 21 Sep 2026 20:54:41 +0800 you wrote:
> skbmark_decode(), skbprio_decode() and skbtcindex_decode() read fixed-size
> values from the TLV payload without validating its length.
> 
> A malformed IFE frame can declare a shorter payload, causing the decoders
> to consume bytes beyond the declared metadata value:
> 
> [TLV type=IFE_META_SKBMARK len=4]
> -> dlen == 0, but decode reads 4 bytes
> 
> [...]

Here is the summary with links:
  - [net,v2] net/sched: act_ife: validate metadata length before decoding
    https://git.kernel.org/netdev/net/c/d6ec384c87cc

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] 2+ messages in thread

end of thread, other threads:[~2026-09-24 13:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-21 12:54 [PATCH net v2] net/sched: act_ife: validate metadata length before decoding Fang Xieyan
2026-09-24 13:30 ` 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