Skip to contentSkip to conversation

net: add skb_header_pointer_careful() helper

PATCH New Bin Lan 1 participant 2 messages Feb 26 Stable branch

lore
Share & tools
lore

Bin Lan

author
#0 lore raw

PATCH 1/2 net: add skb_header_pointer_careful() helper

From: Eric Dumazet <[email protected]>

[ Upstream commit 13e00fdc9236bd4d0bff4109d2983171fbcb74c4 ]

This variant of skb_header_pointer() should be used in contexts where @offset argument is user-controlled and could be negative.

Negative offsets are supported, as long as the zone starts between skb->head and skb->data.

Signed-off-by Eric Dumazet <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Jakub Kicinski <[email protected]>
[ Adjust context ]
Signed-off-by Bin Lan <[email protected]>
---
 include/linux/skbuff.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)
include/linux/skbuff.h +12 -0
index a8d6e976507e..24ceeab1b509 100644
@@ -3869,6 +3869,18 @@ skb_header_pointer(const struct sk_buff *skb, int offset, int len, void *buffer)
 				    skb_headlen(skb), buffer);
 }
 
+/* Variant of skb_header_pointer() where @offset is user-controlled
+ * and potentially negative.
+ */
+static inline void * __must_check
+skb_header_pointer_careful(const struct sk_buff *skb, int offset,
+			   int len, void *buffer)
+{
+	if (unlikely(offset < 0 && -offset > skb_headroom(skb)))
+		return NULL;
+	return skb_header_pointer(skb, offset, len, buffer);
+}
+
 /**
  *	skb_needs_linearize - check if we need to linearize a given skb
  *			      depending on the given device features.
Signature · 1 line
2.43.0
author
#1 lore raw

PATCH 2/2 net/sched: cls_u32: use skb_header_pointer_careful()

From: Eric Dumazet <[email protected]>

[ Upstream commit cabd1a976375780dabab888784e356f574bbaed8 ]

skb_header_pointer() does not fully validate negative @offset values.

Use skb_header_pointer_careful() instead.

GangMin Kim provided a report and a repro fooling u32_classify():

BUG: KASAN: slab-out-of-bounds in u32_classify+0x1180/0x11b0

net/sched/cls_u32.c:221

Fixes fbc2e7d9cf49 ("cls_u32: use skb_header_pointer() to dereference data safely")
Reported-by GangMin Kim <[email protected]>
Closes https://lore.kernel.org/netdev/CANn89iJkyUZ=mAzLzC4GdcAgLuPnUoivdLaOs6B9rq5_erj76w@mail.gmail.com/T/ open here
Signed-off-by Eric Dumazet <[email protected]>
Link https://patch.msgid.link/[email protected] open here
Signed-off-by Jakub Kicinski <[email protected]>
Signed-off-by Bin Lan <[email protected]>
---
 net/sched/cls_u32.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
net/sched/cls_u32.c +6 -7
index 440c29dc058e..23f9e2b5f3b6 100644
@@ -149,10 +149,8 @@ static int u32_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 			int toff = off + key->off + (off2 & key->offmask);
 			__be32 *data, hdata;
 
-			if (skb_headroom(skb) + toff > INT_MAX)
-				goto out;
-
-			data = skb_header_pointer(skb, toff, 4, &hdata);
+			data = skb_header_pointer_careful(skb, toff, 4,
+							  &hdata);
 			if (!data)
 				goto out;
 			if ((*data ^ key->val) & key->mask) {

@@ -202,8 +200,9 @@ static int u32_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 		if (ht->divisor) {
 			__be32 *data, hdata;
 
-			data = skb_header_pointer(skb, off + n->sel.hoff, 4,
-						  &hdata);
+			data = skb_header_pointer_careful(skb,
+							  off + n->sel.hoff,
+							  4, &hdata);
 			if (!data)
 				goto out;
 			sel = ht->divisor & u32_hash_fold(*data, &n->sel,

@@ -217,7 +216,7 @@ static int u32_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 			if (n->sel.flags & TC_U32_VAROFFSET) {
 				__be16 *data, hdata;
 
-				data = skb_header_pointer(skb,
+				data = skb_header_pointer_careful(skb,
 							  off + n->sel.offoff,
 							  2, &hdata);
 				if (!data)
Signature · 1 line
2.43.0