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@xxxxxxxxx>
> > Reported-by: Junrui Luo <moonafterrain@xxxxxxxxxxx>
> > Fixes: 663ead3bb8d5 ("[NET]: Use csum_start offset instead of skb_transport_header")
> > Signed-off-by: Junrui Luo <moonafterrain@xxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxx>