[PATCH net 0/2] net: fix potential crash in net/sched/cls_u32.c

[PATCH net 0/2] net: fix potential crash in net/sched/cls_u32.c

[Eric Dumazet]

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

Add skb_header_pointer_careful() variant of skb_header_pointer()
and use it in net/sched/cls_u32.c.

Later we can also use it in net/sched/act_pedit.c

Eric Dumazet (2):
  net: add skb_header_pointer_careful() helper
  net/sched: cls_u32: use skb_header_pointer_careful()

 include/linux/skbuff.h | 12 ++++++++++++
 net/sched/cls_u32.c    | 13 ++++++-------
 2 files changed, 18 insertions(+), 7 deletions(-)

-- 
2.53.0.rc1.217.geba53bf80e-goog

[PATCH net 1/2] net: add skb_header_pointer_careful() helper

[Eric Dumazet]

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 <edumazet@google.com>
---
 include/linux/skbuff.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 86737076101d4a8452e90fe78adcdcfdefb79169..72679aa7af587cad524feba82432154cfb8afbd3 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4301,6 +4301,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_headlen(skb)))
+		return NULL;
+	return skb_header_pointer(skb, offset, len, buffer);
+}
+
 static inline void * __must_check
 skb_pointer_if_linear(const struct sk_buff *skb, int offset, int len)
 {
-- 
2.53.0.rc1.217.geba53bf80e-goog

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

[Eric Dumazet]

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

Many many thanks to GangMin!

Fixes: fbc2e7d9cf49 ("cls_u32: use skb_header_pointer() to dereference data safely")
Reported-by: GangMin Kim <km.kim1503@gmail.com>
Closes: https://lore.kernel.org/netdev/CANn89iJkyUZ=mAzLzC4GdcAgLuPnUoivdLaOs6B9rq5_erj76w@mail.gmail.com/T/
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/cls_u32.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 2a1c00048fd6f4b700bee11c80502aa3ff993331..58e849c0acf412d3a5ed5cd9a2b32929a720cb17 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -161,10 +161,8 @@ TC_INDIRECT_SCOPE int u32_classify(struct sk_buff *skb,
 			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) {
@@ -214,8 +212,9 @@ TC_INDIRECT_SCOPE int u32_classify(struct sk_buff *skb,
 		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,
@@ -229,7 +228,7 @@ TC_INDIRECT_SCOPE int u32_classify(struct sk_buff *skb,
 			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)
-- 
2.53.0.rc1.217.geba53bf80e-goog

Re: [net,1/2] net: add skb_header_pointer_careful() helper

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: add skb_header_pointer_careful() helper

This patch adds a new helper to validate negative offsets before calling
skb_header_pointer().  The commit message states that negative offsets
are supported "as long as the zone starts between skb->head and
skb->data."

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 86737076101d..72679aa7af58 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -4301,6 +4301,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_headlen(skb)))
> +		return NULL;

Should this check use skb_headroom() instead of skb_headlen()?

The commit message says negative offsets should be valid when the zone
is "between skb->head and skb->data" - that describes the headroom, not
the headlen.  For a negative offset, skb_header_pointer() returns
skb->data + offset, which points into the headroom area.

skb_headlen() returns the linear data length after skb->data (skb->len
minus skb->data_len), while skb_headroom() returns the space before
skb->data (skb->data minus skb->head).

These values are quite different in practice - headroom is typically
small (2-128 bytes), while headlen can be up to MTU size (~1500 bytes).
With the current check, an offset like -100 would pass validation when
skb_headlen is 1500, but could read before skb->head if skb_headroom is
only 2 bytes.

The existing offset_valid() function in net/sched/act_pedit.c uses
skb_headroom() for this same type of check:

    if  (offset < 0 && -offset > skb_headroom(skb))
        return false;

> +	return skb_header_pointer(skb, offset, len, buffer);
> +}
-- 
pw-bot: cr

Re: [net,1/2] net: add skb_header_pointer_careful() helper

[Eric Dumazet]

On Wed, Jan 28, 2026 at 3:56 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: add skb_header_pointer_careful() helper
>
> This patch adds a new helper to validate negative offsets before calling
> skb_header_pointer().  The commit message states that negative offsets
> are supported "as long as the zone starts between skb->head and
> skb->data."
>
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 86737076101d..72679aa7af58 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -4301,6 +4301,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_headlen(skb)))
> > +             return NULL;
>
> Should this check use skb_headroom() instead of skb_headlen()?
>
> The commit message says negative offsets should be valid when the zone
> is "between skb->head and skb->data" - that describes the headroom, not
> the headlen.  For a negative offset, skb_header_pointer() returns
> skb->data + offset, which points into the headroom area.
>
> skb_headlen() returns the linear data length after skb->data (skb->len
> minus skb->data_len), while skb_headroom() returns the space before
> skb->data (skb->data minus skb->head).
>
> These values are quite different in practice - headroom is typically
> small (2-128 bytes), while headlen can be up to MTU size (~1500 bytes).
> With the current check, an offset like -100 would pass validation when
> skb_headlen is 1500, but could read before skb->head if skb_headroom is
> only 2 bytes.
>
> The existing offset_valid() function in net/sched/act_pedit.c uses
> skb_headroom() for this same type of check:
>
>     if  (offset < 0 && -offset > skb_headroom(skb))
>         return false;
>
> > +     return skb_header_pointer(skb, offset, len, buffer);
> > +}

Yes, of course, silly error on my side.