[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGfirfcWbnW0wMYSZNs4M5Hj-qPn1edJsDq9Y-4yxuBvOMzdWQ@mail.gmail.com>
Date: Tue, 27 Jan 2026 08:47:54 +0900
From: GangMin Kim <km.kim1503@...il.com>
To: Jamal Hadi Salim <jhs@...atatu.com>, Eric Dumazet <edumazet@...gle.com>
Cc: davem@...emloft.net, jiri@...nulli.us, kuba@...nel.org,
netdev@...r.kernel.org, pabeni@...hat.com, xiyou.wangcong@...il.com,
horms@...nel.org, syzkaller@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [BUG] KASAN: slab-use-after-free Read in u32_classify
Structured the patch following similar commits to prevent out-of-bounds access.
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -111,6 +111,17 @@ static inline unsigned int u32_hash_fold(__be32 key,
return h;
}
+static bool offset_valid(struct sk_buff *skb, int offset)
+{
+ if (offset > 0 && offset > skb->len)
+ return false;
+
+ if (offset < 0 && -offset > skb_headroom(skb))
+ return false;
+
+ return true;
+}
+
TC_INDIRECT_SCOPE int u32_classify(struct sk_buff *skb,
const struct tcf_proto *tp,
struct tcf_result *res)
@@ -214,6 +225,8 @@ TC_INDIRECT_SCOPE int u32_classify(struct sk_buff *skb,
if (ht->divisor) {
__be32 *data, hdata;
+ if (!offset_valid(skb, off + n->sel.hoff))
+ goto out;
data = skb_header_pointer(skb, off + n->sel.hoff, 4,
&hdata);
if (!data)
@@ -229,6 +242,8 @@ TC_INDIRECT_SCOPE int u32_classify(struct sk_buff *skb,
if (n->sel.flags & TC_U32_VAROFFSET) {
__be16 *data, hdata;
+ if (!offset_valid(skb, off + n->sel.offoff))
+ goto out;
data = skb_header_pointer(skb,
off + n->sel.offoff,
2, &hdata);
2026년 1월 27일 (화) AM 12:24, Jamal Hadi Salim <jhs@...atatu.com>님이 작성:
>
> On Mon, Jan 26, 2026 at 9:11 AM Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > On Mon, Jan 26, 2026 at 2:51 PM Jamal Hadi Salim <jhs@...atatu.com> wrote:
> > >
> > > On Mon, Jan 26, 2026 at 7:26 AM Eric Dumazet <edumazet@...gle.com> wrote:
> > > >
> > > > On Mon, Jan 26, 2026 at 1:11 PM 김강민 <km.kim1503@...il.com> wrote:
> > > > >
> > > > > I propose the following patch and would appreciate your review.
> > > > >
> > > > > hoff is used as an offset argument to skb_header_pointer() in
> > > > > u32_classify(), and negative values can cause out-of-bounds read. As
> > > > > shown in the code below, offoff follows the same pattern and requires
> > > > > the same validation.
> > > > > ```
> > > > > TC_INDIRECT_SCOPE int u32_classify(struct sk_buff *skb,
> > > > > const struct tcf_proto *tp,
> > > > > struct tcf_result *res)
> > > > > {
> > > > > ...
> > > > > if (n) {
> > > > > ...
> > > > > if (n->sel.flags & (TC_U32_OFFSET | TC_U32_VAROFFSET)) {
> > > > > off2 = n->sel.off + 3;
> > > > > if (n->sel.flags & TC_U32_VAROFFSET) {
> > > > > __be16 *data, hdata;
> > > > >
> > > > > data = skb_header_pointer(skb,
> > > > > off + n->sel.offoff,
> > > > > 2, &hdata);
> > > > > if (!data)
> > > > > goto out;
> > > > > off2 += ntohs(n->sel.offmask & *data) >>
> > > > > n->sel.offshift;
> > > > > }
> > > > > off2 &= ~3;
> > > > > }
> > > > > ...
> > > > > }
> > > > > ...
> > > > > }
> > > > > ```
> > > > > Therefore, I propose a patch that rejects negative hoff and offoff
> > > > > values during u32 classifier configuration.
> > > > >
> > > > > --- a/net/sched/cls_u32.c
> > > > > +++ b/net/sched/cls_u32.c
> > > > > @@ -1104,6 +1104,11 @@ static int u32_change(struct net *net, struct
> > > > > sk_buff *in_skb,
> > > > > err = -EINVAL;
> > > > > goto erridr;
> > > > > }
> > > > > +
> > > > > + if (s->hoff < 0 || s->offoff < 0) {
> > > > > + err = -EINVAL;
> > > > > + goto erridr;
> > > > > + }
> > > > >
> > > >
> > > > Negative values are valid, it is ok to access data between skb->head
> > > > and skb->data.
> > >
> > > Right. We have to do this at runtime in classify().
> > > Can you test with your POC with Eric's suggested fix?
> > >
> > > Eric, can we do something like pedit? See: offset_valid() usage
> >
> >
> > Yes, it seems net/sched/act_pedit.c had a similar issue in the past :)
> > Amir fixed it with
> >
> > commit 95c2027bfeda21a28eb245121e6a249f38d0788e
> > Author: Amir Vadai <amir@...ai.me>
> > Date: Mon Nov 28 12:56:40 2016 +0200
> >
> > net/sched: pedit: make sure that offset is valid
> >
> > Add a validation function to make sure offset is valid:
> > 1. Not below skb head (could happen when offset is negative).
> > 2. Validate both 'offset' and 'at'.
> >
> > Signed-off-by: Amir Vadai <amir@...ai.me>
> > Signed-off-by: David S. Miller <davem@...emloft.net>
>
> 김강민,
> Are you able to send a similar patch? Otherwise one of us will.
>
> cheers,
> jamal
Powered by blists - more mailing lists