[PATCH net] skb_checksum_help: fix out-of-bounds access

[PATCH net] skb_checksum_help: fix out-of-bounds access

[Junrui Luo]

The skb_checksum_help() function does not validate negative offset
values returned by skb_checksum_start_offset(). This can occur when
__skb_pull() is called on a packet, increasing the headroom while
leaving csum_start unchanged.

A negative offset causes out-of-bounds memory access:
- skb_checksum() reads before skb->data when computing the checksum
- skb_checksum_help() writes before skb->data

Add validation to detect and reject negative offsets.

Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Reported-by: Junrui Luo <moonafterrain@outlook.com>
Fixes: 663ead3bb8d5 ("[NET]: Use csum_start offset instead of skb_transport_header")
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 net/core/dev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 9094c0fb8c68..30161b9240a2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3574,6 +3574,11 @@ int skb_checksum_help(struct sk_buff *skb)
 
 	offset = skb_checksum_start_offset(skb);
 	ret = -EINVAL;
+	if (unlikely(offset < 0)) {
+		DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
+		WARN_ONCE(true, "offset (%d) < 0\n", offset);
+		goto out;
+	}
 	if (unlikely(offset >= skb_headlen(skb))) {
 		DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
 		WARN_ONCE(true, "offset (%d) >= skb_headlen() (%u)\n",

---
base-commit: cfd4039213e7b5a828c5b78e1b5235cac91af53d
change-id: 20251210-fixes-ef9fa1c91916

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH net] skb_checksum_help: fix out-of-bounds access

[Willem de Bruijn]

Junrui Luo wrote:
> The skb_checksum_help() function does not validate negative offset
> values returned by skb_checksum_start_offset(). This can occur when
> __skb_pull() is called on a packet, increasing the headroom while
> leaving csum_start unchanged.

Do you have a specific example where this happens?
 
> A negative offset causes out-of-bounds memory access:
> - skb_checksum() reads before skb->data when computing the checksum
> - skb_checksum_help() writes before skb->data

I don't think this is true out-of-bounds as long as the data access
starts greater than or equal to skb->head, which it will.

There are known cases where such negative skb offsets are
intentional. I don't think this is one of them, but needs a careful
analysis.

The use in skb_checksum does seem to indicate that this is not
intentional indeed. Checksumming a packet where the L4 header would
lie outside the data.

    csum = skb_checksum(skb, offset, skb->len - offset, 0);

> Add validation to detect and reject negative offsets.
> 
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Reported-by: Junrui Luo <moonafterrain@outlook.com>
> Fixes: 663ead3bb8d5 ("[NET]: Use csum_start offset instead of skb_transport_header")
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
>  net/core/dev.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 9094c0fb8c68..30161b9240a2 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3574,6 +3574,11 @@ int skb_checksum_help(struct sk_buff *skb)
>  
>  	offset = skb_checksum_start_offset(skb);
>  	ret = -EINVAL;
> +	if (unlikely(offset < 0)) {
> +		DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
> +		WARN_ONCE(true, "offset (%d) < 0\n", offset);
> +		goto out;
> +	}
>  	if (unlikely(offset >= skb_headlen(skb))) {
>  		DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
>  		WARN_ONCE(true, "offset (%d) >= skb_headlen() (%u)\n",
> 
> ---
> base-commit: cfd4039213e7b5a828c5b78e1b5235cac91af53d
> change-id: 20251210-fixes-ef9fa1c91916
> 
> Best regards,
> -- 
> Junrui Luo <moonafterrain@outlook.com>
> 

Re: [PATCH net] skb_checksum_help: fix out-of-bounds access

[Junrui Luo]

On Wednesday 10 December 2025 09:55:17 PM (+08:00), Willem de Bruijn wrote:

> Junrui Luo wrote:
> > The skb_checksum_help() function does not validate negative offset
> > values returned by skb_checksum_start_offset(). This can occur when
> > __skb_pull() is called on a packet, increasing the headroom while
> > leaving csum_start unchanged.
>
> Do you have a specific example where this happens?

After testing, I found that triggering this condition in practice is
difficult. In my test cases, normal packet processing does not create
the conditions where headroom becomes large enough to make the offset
negative.

>
> > A negative offset causes out-of-bounds memory access:
> > - skb_checksum() reads before skb->data when computing the checksum
> > - skb_checksum_help() writes before skb->data
>
> I don't think this is true out-of-bounds as long as the data access
> starts greater than or equal to skb->head, which it will.

I agree that it is not strictly out-of-bounds, but rather an incorrect
memory access.

>
> There are known cases where such negative skb offsets are
> intentional. I don't think this is one of them, but needs a careful
> analysis.
>
> The use in skb_checksum does seem to indicate that this is not
> intentional indeed. Checksumming a packet where the L4 header would
> lie outside the data.
>
> csum = skb_checksum(skb, offset, skb->len - offset, 0);
>

Since this is not intentional indeed, I think we should apply a more
complete bounds check. Even though the negative offset is unlikely to
happen.

> > Add validation to detect and reject negative offsets.
> >
> > Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> > Reported-by: Junrui Luo <moonafterrain@outlook.com>
> > Fixes: 663ead3bb8d5 ("[NET]: Use csum_start offset instead of skb_transport_header")
> > Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> > ---
> > net/core/dev.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 9094c0fb8c68..30161b9240a2 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -3574,6 +3574,11 @@ int skb_checksum_help(struct sk_buff *skb)
> >
> > offset = skb_checksum_start_offset(skb);
> > ret = -EINVAL;
> > + if (unlikely(offset < 0)) {
> > + DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
> > + WARN_ONCE(true, "offset (%d) < 0\n", offset);
> > + goto out;
> > + }
> > if (unlikely(offset >= skb_headlen(skb))) {
> > DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
> > WARN_ONCE(true, "offset (%d) >= skb_headlen() (%u)\n",
> >
> > ---
> > base-commit: cfd4039213e7b5a828c5b78e1b5235cac91af53d
> > change-id: 20251210-fixes-ef9fa1c91916
> >
> > Best regards,
> > --
> > Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH net] skb_checksum_help: fix out-of-bounds access

[Eric Dumazet]

On Fri, Dec 12, 2025 at 4:30 AM Junrui Luo <moonafterrain@outlook.com> wrote:
>
> On Wednesday 10 December 2025 09:55:17 PM (+08:00), Willem de Bruijn wrote:
>
> > Junrui Luo wrote:
> > > The skb_checksum_help() function does not validate negative offset
> > > values returned by skb_checksum_start_offset(). This can occur when
> > > __skb_pull() is called on a packet, increasing the headroom while
> > > leaving csum_start unchanged.
> >
> > Do you have a specific example where this happens?
>
> After testing, I found that triggering this condition in practice is
> difficult. In my test cases, normal packet processing does not create
> the conditions where headroom becomes large enough to make the offset
> negative.

I suspect this is virtio fed packet ?

Adding WARN_ONCE(true, "offset (%d) < 0\n", offset) will still trigger
bugs as far as syzbot is concerned.

BTW, the current code following your added code should catch the bug the same,
so your patch makes no difference ?

      if (unlikely(offset >= skb_headlen(skb))) {
                DO_ONCE_LITE(skb_dump, KERN_ERR, skb, false);
                WARN_ONCE(true, "offset (%d) >= skb_headlen() (%u)\n",

Because offset is promoted to "unsigned int", as skb_headlen() is "unsigned int"

Look at commits eeee4b77dc52b ("net: add more debug info in
skb_checksum_help()")
and 26c29961b1424 ("net: refine debug info in skb_checksum_help()")