* [PATCH net-next 0/6] net/sched: netem: cleanups and improvements
From: Stephen Hemminger @ 2026-03-28 18:26 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger
Cleanup and improvement patches for netem, done as follow-on
to the bug fix series found during AI-assisted review.
- replace pr_info() with netlink extack error reporting
- validate slot min/max delay range on configuration
- fix slot delay calculation overflow for large ranges
- remove unused loss model struct fields
- remove stale VERSION string
- add per-impairment extended statistics (delayed, dropped,
corrupted, duplicated, reordered, ecn_marked)
The xstats patch requires a corresponding iproute2 change
to display the new counters in tc -s qdisc show. This will go
as separate patch after this.
Stephen Hemminger (6):
net/sched: netem: replace pr_info with netlink extack error messages
net/sched: netem: check for invalid slot range
net/sched: netem: fix slot delay calculation overflow
net/sched: netem: remove unused loss model fields
net/sched: netem: remove useless VERSION
net/sched: netem: add per-impairment extended statistics
include/uapi/linux/pkt_sched.h | 9 +++
net/sched/sch_netem.c | 120 ++++++++++++++++++++++-----------
2 files changed, 89 insertions(+), 40 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH net-next 1/6] net/sched: netem: replace pr_info with netlink extack error messages
From: Stephen Hemminger @ 2026-03-28 18:26 UTC (permalink / raw)
To: netdev
Cc: Stephen Hemminger, Jamal Hadi Salim, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
open list
In-Reply-To: <20260328182704.456993-1-stephen@networkplumber.org>
netem predates the netlink extended ack mechanism and uses pr_info()
to report configuration errors. These messages go to the kernel log
where the user running tc may never see them, and in unprivileged
user namespace contexts they can be used for log spam.
Replace pr_info() with NL_SET_ERR_MSG() and NL_SET_ERR_MSG_FMT()
which return error details to the caller via netlink.
Remove the uninformative "netem: change failed" message from netem_init()
since the error is already propagated.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/sch_netem.c | 42 ++++++++++++++++++++++++------------------
1 file changed, 24 insertions(+), 18 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 5de1c932944a..73d0e85eeadc 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -804,19 +804,24 @@ static void dist_free(struct disttable *d)
* signed 16 bit values.
*/
-static int get_dist_table(struct disttable **tbl, const struct nlattr *attr)
+static int get_dist_table(struct disttable **tbl, const struct nlattr *attr,
+ struct netlink_ext_ack *extack)
{
size_t n = nla_len(attr)/sizeof(__s16);
const __s16 *data = nla_data(attr);
struct disttable *d;
int i;
- if (!n || n > NETEM_DIST_MAX)
+ if (!n || n > NETEM_DIST_MAX) {
+ NL_SET_ERR_MSG(extack, "invalid distribution table length");
return -EINVAL;
+ }
d = kvmalloc_flex(*d, table, n);
- if (!d)
+ if (!d) {
+ NL_SET_ERR_MSG(extack, "allocation of distribution table failed");
return -ENOMEM;
+ }
d->size = n;
for (i = 0; i < n; i++)
@@ -887,7 +892,8 @@ static void get_rate(struct netem_sched_data *q, const struct nlattr *attr)
q->cell_size_reciprocal = (struct reciprocal_value) { 0 };
}
-static int get_loss_clg(struct netem_sched_data *q, const struct nlattr *attr)
+static int get_loss_clg(struct netem_sched_data *q, const struct nlattr *attr,
+ struct netlink_ext_ack *extack)
{
const struct nlattr *la;
int rem;
@@ -900,7 +906,7 @@ static int get_loss_clg(struct netem_sched_data *q, const struct nlattr *attr)
const struct tc_netem_gimodel *gi = nla_data(la);
if (nla_len(la) < sizeof(struct tc_netem_gimodel)) {
- pr_info("netem: incorrect gi model size\n");
+ NL_SET_ERR_MSG(extack, "incorrect gi model size");
return -EINVAL;
}
@@ -919,7 +925,7 @@ static int get_loss_clg(struct netem_sched_data *q, const struct nlattr *attr)
const struct tc_netem_gemodel *ge = nla_data(la);
if (nla_len(la) < sizeof(struct tc_netem_gemodel)) {
- pr_info("netem: incorrect ge model size\n");
+ NL_SET_ERR_MSG(extack, "incorrect ge model size");
return -EINVAL;
}
@@ -933,7 +939,7 @@ static int get_loss_clg(struct netem_sched_data *q, const struct nlattr *attr)
}
default:
- pr_info("netem: unknown loss type %u\n", type);
+ NL_SET_ERR_MSG_FMT(extack, "unknown loss type %u", type);
return -EINVAL;
}
}
@@ -956,12 +962,13 @@ static const struct nla_policy netem_policy[TCA_NETEM_MAX + 1] = {
};
static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla,
- const struct nla_policy *policy, int len)
+ const struct nla_policy *policy, size_t len,
+ struct netlink_ext_ack *extack)
{
int nested_len = nla_len(nla) - NLA_ALIGN(len);
if (nested_len < 0) {
- pr_info("netem: invalid attributes len %d\n", nested_len);
+ NL_SET_ERR_MSG(extack, "invalid attribute len");
return -EINVAL;
}
@@ -1010,8 +1017,7 @@ static int check_netem_in_tree(struct Qdisc *sch, bool duplicates,
}
/* Parse netlink message to set options */
-static int netem_change(struct Qdisc *sch, struct nlattr *opt,
- struct netlink_ext_ack *extack)
+static int netem_change(struct Qdisc *sch, struct nlattr *opt, struct netlink_ext_ack *extack)
{
struct netem_sched_data *q = qdisc_priv(sch);
struct nlattr *tb[TCA_NETEM_MAX + 1];
@@ -1023,18 +1029,18 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
int ret;
qopt = nla_data(opt);
- ret = parse_attr(tb, TCA_NETEM_MAX, opt, netem_policy, sizeof(*qopt));
+ ret = parse_attr(tb, TCA_NETEM_MAX, opt, netem_policy, sizeof(*qopt), extack);
if (ret < 0)
return ret;
if (tb[TCA_NETEM_DELAY_DIST]) {
- ret = get_dist_table(&delay_dist, tb[TCA_NETEM_DELAY_DIST]);
+ ret = get_dist_table(&delay_dist, tb[TCA_NETEM_DELAY_DIST], extack);
if (ret)
goto table_free;
}
if (tb[TCA_NETEM_SLOT_DIST]) {
- ret = get_dist_table(&slot_dist, tb[TCA_NETEM_SLOT_DIST]);
+ ret = get_dist_table(&slot_dist, tb[TCA_NETEM_SLOT_DIST], extack);
if (ret)
goto table_free;
}
@@ -1045,7 +1051,7 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt,
old_loss_model = q->loss_model;
if (tb[TCA_NETEM_LOSS]) {
- ret = get_loss_clg(q, tb[TCA_NETEM_LOSS]);
+ ret = get_loss_clg(q, tb[TCA_NETEM_LOSS], extack);
if (ret) {
q->loss_model = old_loss_model;
q->clg = old_clg;
@@ -1134,13 +1140,13 @@ static int netem_init(struct Qdisc *sch, struct nlattr *opt,
qdisc_watchdog_init(&q->watchdog, sch);
- if (!opt)
+ if (!opt) {
+ NL_SET_ERR_MSG(extack, "missing parameters");
return -EINVAL;
+ }
q->loss_model = CLG_RANDOM;
ret = netem_change(sch, opt, extack);
- if (ret)
- pr_info("netem: change failed\n");
return ret;
}
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 2/6] net/sched: netem: check for invalid slot range
From: Stephen Hemminger @ 2026-03-28 18:26 UTC (permalink / raw)
To: netdev
Cc: Stephen Hemminger, Jamal Hadi Salim, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Neal Cardwell, Yousuk Seung, open list
In-Reply-To: <20260328182704.456993-1-stephen@networkplumber.org>
Reject slot configuration where min_delay exceeds max_delay.
The delay range computation in get_slot_next() underflows in
this case, producing bogus results.
Fixes: 0a9fe5c375b5 ("netem: slotting with non-uniform distribution")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/sch_netem.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 73d0e85eeadc..de22d754cb79 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -831,6 +831,17 @@ static int get_dist_table(struct disttable **tbl, const struct nlattr *attr,
return 0;
}
+static int validate_slot(const struct nlattr *attr, struct netlink_ext_ack *extack)
+{
+ const struct tc_netem_slot *c = nla_data(attr);
+
+ if (c->min_delay > c->max_delay) {
+ NL_SET_ERR_MSG(extack, "slot min delay greater than max delay");
+ return -EINVAL;
+ }
+ return 0;
+}
+
static void get_slot(struct netem_sched_data *q, const struct nlattr *attr)
{
const struct tc_netem_slot *c = nla_data(attr);
@@ -1045,6 +1056,12 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt, struct netlink_ex
goto table_free;
}
+ if (tb[TCA_NETEM_SLOT]) {
+ ret = validate_slot(tb[TCA_NETEM_SLOT], extack);
+ if (ret)
+ goto table_free;
+ }
+
sch_tree_lock(sch);
/* backup q->clg and q->loss_model */
old_clg = q->clg;
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 3/6] net/sched: netem: fix slot delay calculation overflow
From: Stephen Hemminger @ 2026-03-28 18:26 UTC (permalink / raw)
To: netdev
Cc: Stephen Hemminger, Jamal Hadi Salim, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Yousuk Seung, Neal Cardwell, open list
In-Reply-To: <20260328182704.456993-1-stephen@networkplumber.org>
get_slot_next() computes a random delay between min_delay and
max_delay using:
get_random_u32() * (max_delay - min_delay) >> 32
This overflows signed 64-bit arithmetic when the delay range exceeds
approximately 2.1 seconds (2^31 nanoseconds), producing a negative
result that effectively disables slot-based pacing. This is a
realistic configuration for WAN emulation (e.g., slot 1s 5s).
Use mul_u64_u32_shr() which handles the widening multiply without
overflow.
Fixes: 0a9fe5c375b5 ("netem: slotting with non-uniform distribution")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/sch_netem.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index de22d754cb79..69c93f7ade62 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -658,9 +658,8 @@ static void get_slot_next(struct netem_sched_data *q, u64 now)
if (!q->slot_dist)
next_delay = q->slot_config.min_delay +
- (get_random_u32() *
- (q->slot_config.max_delay -
- q->slot_config.min_delay) >> 32);
+ mul_u64_u32_shr(q->slot_config.max_delay - q->slot_config.min_delay,
+ get_random_u32(), 32);
else
next_delay = tabledist(q->slot_config.dist_delay,
(s32)(q->slot_config.dist_jitter),
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 4/6] net/sched: netem: remove unused loss model fields
From: Stephen Hemminger @ 2026-03-28 18:26 UTC (permalink / raw)
To: netdev
Cc: Stephen Hemminger, Jamal Hadi Salim, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
open list
In-Reply-To: <20260328182704.456993-1-stephen@networkplumber.org>
The _4_state_model and GE_state_model enum definitions are declared
as struct members but are never read or written. Only the enum
constants they define (TX_IN_GAP_PERIOD, GOOD_STATE, etc.) are used.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/sch_netem.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 69c93f7ade62..87caf1b9a4a7 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -71,6 +71,18 @@ struct disttable {
s16 table[] __counted_by(size);
};
+enum GE_state_model {
+ GOOD_STATE = 1,
+ BAD_STATE,
+};
+
+enum _4_state_model {
+ TX_IN_GAP_PERIOD = 1,
+ TX_IN_BURST_PERIOD,
+ LOST_IN_GAP_PERIOD,
+ LOST_IN_BURST_PERIOD,
+};
+
struct netem_sched_data {
/* internal t(ime)fifo qdisc uses t_root and sch->limit */
struct rb_root t_root;
@@ -121,18 +133,6 @@ struct netem_sched_data {
CLG_GILB_ELL,
} loss_model;
- enum {
- TX_IN_GAP_PERIOD = 1,
- TX_IN_BURST_PERIOD,
- LOST_IN_GAP_PERIOD,
- LOST_IN_BURST_PERIOD,
- } _4_state_model;
-
- enum {
- GOOD_STATE = 1,
- BAD_STATE,
- } GE_state_model;
-
/* Correlated Loss Generation models */
struct clgstate {
/* state of the Markov chain */
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 5/6] net/sched: netem: remove useless VERSION
From: Stephen Hemminger @ 2026-03-28 18:26 UTC (permalink / raw)
To: netdev
Cc: Stephen Hemminger, Jamal Hadi Salim, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
open list
In-Reply-To: <20260328182704.456993-1-stephen@networkplumber.org>
The version printed was never updated and kernel version is
better indication of what is fixed or not.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
net/sched/sch_netem.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 87caf1b9a4a7..26fc68e34b91 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -27,8 +27,6 @@
#include <net/pkt_sched.h>
#include <net/inet_ecn.h>
-#define VERSION "1.3"
-
/* Network Emulation Queuing algorithm.
====================================
@@ -1379,16 +1377,15 @@ static struct Qdisc_ops netem_qdisc_ops __read_mostly = {
};
MODULE_ALIAS_NET_SCH("netem");
-
static int __init netem_module_init(void)
{
- pr_info("netem: version " VERSION "\n");
return register_qdisc(&netem_qdisc_ops);
}
static void __exit netem_module_exit(void)
{
unregister_qdisc(&netem_qdisc_ops);
}
+
module_init(netem_module_init)
module_exit(netem_module_exit)
MODULE_LICENSE("GPL");
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 6/6] net/sched: netem: add per-impairment extended statistics
From: Stephen Hemminger @ 2026-03-28 18:26 UTC (permalink / raw)
To: netdev
Cc: Stephen Hemminger, Jamal Hadi Salim, Jiri Pirko, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
open list
In-Reply-To: <20260328182704.456993-1-stephen@networkplumber.org>
netem applies several impairments (delay, loss, corruption, duplication,
reordering) but exposes no counters distinguishing which impairment
affected a given packet.
Add a struct tc_netem_xstats reported via TCA_STATS_APP so that
userspace (tc -s qdisc show) can display per-impairment counters.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
include/uapi/linux/pkt_sched.h | 9 +++++++++
net/sched/sch_netem.c | 27 ++++++++++++++++++++++++---
2 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 66e8072f44df..fada10cb9b7b 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -569,6 +569,15 @@ struct tc_netem_gemodel {
#define NETEM_DIST_SCALE 8192
#define NETEM_DIST_MAX 16384
+struct tc_netem_xstats {
+ __u32 delayed; /* packets delayed */
+ __u32 dropped; /* packets dropped by loss model */
+ __u32 corrupted; /* packets with bit errors injected */
+ __u32 duplicated; /* duplicate packets generated */
+ __u32 reordered; /* packets sent out of order */
+ __u32 ecn_marked; /* packets ECN CE-marked (not dropped)*/
+};
+
/* DRR */
enum {
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 26fc68e34b91..755e8d009f85 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -152,6 +152,9 @@ struct netem_sched_data {
} slot;
struct disttable *slot_dist;
+
+ /* Per-impairment counters */
+ struct tc_netem_xstats xstats;
};
/* Time stamp put into socket buffer control block
@@ -459,17 +462,22 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
skb->prev = NULL;
/* Random duplication */
- if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor, &q->prng))
+ if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor, &q->prng)) {
++count;
+ q->xstats.duplicated++;
+ }
/* Drop packet? */
if (loss_event(q)) {
- if (q->ecn && INET_ECN_set_ce(skb))
+ if (q->ecn && INET_ECN_set_ce(skb)) {
qdisc_qstats_drop(sch); /* mark packet */
- else
+ q->xstats.ecn_marked++;
+ } else {
--count;
+ }
}
if (count == 0) {
+ q->xstats.dropped++;
qdisc_qstats_drop(sch);
__qdisc_drop(skb, to_free);
return NET_XMIT_SUCCESS | __NET_XMIT_BYPASS;
@@ -495,6 +503,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
* do it now in software before we mangle it.
*/
if (q->corrupt && q->corrupt >= get_crandom(&q->corrupt_cor, &q->prng)) {
+ q->xstats.corrupted++;
if (skb_is_gso(skb)) {
skb = netem_segment(skb, sch, to_free);
if (!skb)
@@ -597,6 +606,9 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
delay += packet_time_ns(qdisc_pkt_len(skb), q);
}
+ if (delay > 0)
+ q->xstats.delayed++;
+
cb->time_to_send = now + delay;
++q->counter;
tfifo_enqueue(skb, sch);
@@ -605,6 +617,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
* Do re-ordering by putting one out of N packets at the front
* of the queue.
*/
+ q->xstats.reordered++;
cb->time_to_send = ktime_get_ns();
q->counter = 0;
@@ -1311,6 +1324,13 @@ static int netem_dump(struct Qdisc *sch, struct sk_buff *skb)
return -1;
}
+static int netem_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
+{
+ struct netem_sched_data *q = qdisc_priv(sch);
+
+ return gnet_stats_copy_app(d, &q->xstats, sizeof(q->xstats));
+}
+
static int netem_dump_class(struct Qdisc *sch, unsigned long cl,
struct sk_buff *skb, struct tcmsg *tcm)
{
@@ -1373,6 +1393,7 @@ static struct Qdisc_ops netem_qdisc_ops __read_mostly = {
.destroy = netem_destroy,
.change = netem_change,
.dump = netem_dump,
+ .dump_stats = netem_dump_stats,
.owner = THIS_MODULE,
};
MODULE_ALIAS_NET_SCH("netem");
--
2.53.0
^ permalink raw reply related
* Re: [PATCH net-next 2/2] net: stmmac: simplify GSO/TSO test in stmmac_xmit()
From: Russell King (Oracle) @ 2026-03-28 18:55 UTC (permalink / raw)
To: Andrew Lunn, Ong Boon Leong
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-stm32, netdev,
Paolo Abeni
In-Reply-To: <acgPJgW9r0l952qu@shell.armlinux.org.uk>
On Sat, Mar 28, 2026 at 05:25:58PM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 28, 2026 at 09:31:46AM +0000, Russell King (Oracle) wrote:
> > On Sat, Mar 28, 2026 at 08:24:37AM +0000, Russell King (Oracle) wrote:
> > > On Fri, Mar 27, 2026 at 09:40:09AM +0000, Russell King (Oracle) wrote:
> > > > The test in stmmac_xmit() to see whether we should pass the skbuff to
> > > > stmmac_tso_xmit() is more complex than it needs to be. This test can
> > > > be simplified by storing the mask of GSO types that we will pass, and
> > > > setting it according to the enabled features.
> > > >
> > > > Note that "tso" is a mis-nomer since commit b776620651a1 ("net:
> > > > stmmac: Implement UDP Segmentation Offload"). Also note that this
> > > > commit controls both via the TSO feature. We preserve this behaviour
> > > > in this commit.
> > > >
> > > > Also, this commit unconditionally accessed skb_shinfo(skb)->gso_type
> > > > for all frames, even when skb_is_gso() was false. This access is
> > > > eliminated.
> > > >
> > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > >
> > > AI review of this patch regurgitates Jakub's point that was discussed.
> > >
> > > > @@ -3700,7 +3700,7 @@ static int stmmac_hw_setup(struct net_device *dev)
> > > > stmmac_set_rings_length(priv);
> > > >
> > > > /* Enable TSO */
> > > > - if (priv->tso) {
> > > > + if (priv->gso_enabled_types) {
> > > > for (chan = 0; chan < tx_cnt; chan++) {
> > > > struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[chan];
> > > >
> > >
> > > ...
> > >
> > > > @@ -7828,7 +7834,7 @@ static int __stmmac_dvr_probe(struct device *device,
> > > > ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
> > > > if (priv->plat->core_type == DWMAC_CORE_GMAC4)
> > > > ndev->hw_features |= NETIF_F_GSO_UDP_L4;
> > > > - priv->tso = true;
> > > > + stmmac_set_gso_types(priv, true);
> > >
> > > Clearly, the issue it is regurgitating has been there for a long time
> > > and isn't a new issue introduced by this patch.
> > >
> > > AI needs to stop doing this, because it is encouraging multiple changes
> > > in a single patch, which is against the normal kernel process.
> > >
> > > As already pointed out, there are multiple issues with stmmac TSO
> > > support, particularly with glue drivers that enable TSO on some
> > > queues/channels and not others, since netdev core TSO support is
> > > global across all channels.
> > >
> > > So, won't the AI response in this patch - it's just another pre-
> > > existing issue that needs fixing in a separate patch.
> >
> > Looking at the TSO vs TBS issue (which precludes the use of TSO on a
> > channel in stmmac) I can't find an obvious reason for this in the
> > available documentation. However, unfortunately, iMX8MP doesn't support
> > TSO, so the TSO bits are elided there, but does support TBS (needing
> > enhanced descriptors to be enabled). STM32MP151 on the other hand
> > supports TSO but not TBS, and thus fails to mention anything about
> > enhanced descriptors or TBS.
> >
> > When stmmac_enable_tbs() enables TBS, it isn't actually enabling a
> > feature specific bit, but switching the channel to use enhanced
> > descriptor format. This format extends the basic descriptors by
> > placing four extra 32-bit words before the basic descriptor.
> >
> > Looking at the enhanced normal descriptor format for TDES3, it
> > indicates that the format includes bit 18 in the control field, which
> > is the TSE bit (TCP segmentation enable for this packet.) So, it seems
> > it's not a limitation of the descriptor format.
> >
> > So, either "TSO and TBS cannot co-exist" is incorrect, or there is a
> > hardware limitation that isn't documented between these two manuals.
> >
> > One other interesting point is that stmmac_tso_xmit() seems to
> > handle the case where TSO and TBS are enabled on the channel:
> >
> > if (tx_q->tbs & STMMAC_TBS_AVAIL)
> > mss_desc = &tx_q->dma_entx[tx_q->cur_tx].basic;
> > else
> > mss_desc = &tx_q->dma_tx[tx_q->cur_tx];
> >
> > stmmac_set_mss(priv, mss_desc, mss);
> > ...
> > if (tx_q->tbs & STMMAC_TBS_AVAIL)
> > desc = &tx_q->dma_entx[first_entry].basic;
> > else
> > desc = &tx_q->dma_tx[first_entry];
> > first = desc;
> >
> > etc.
> >
> > Avoiding enabling TSO for a TBS channel was added by this commit:
> >
> > commit 5e6038b88a5718910dd74b949946d9d9cee9a041
> > Author: Ong Boon Leong <boon.leong.ong@intel.com>
> > Date: Wed Apr 21 17:11:49 2021 +0800
> >
> > net: stmmac: fix TSO and TBS feature enabling during driver open
> >
> > TSO and TBS cannot co-exist and current implementation requires two
> > fixes:
> >
> > 1) stmmac_open() does not need to call stmmac_enable_tbs() because
> > the MAC is reset in stmmac_init_dma_engine() anyway.
> > 2) Inside stmmac_hw_setup(), we should call stmmac_enable_tso() for
> > TX Q that is _not_ configured for TBS.
> >
> > Fixes: 579a25a854d4 ("net: stmmac: Initial support for TBS")
> > Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > which doesn't really explain the background, and leaves all the TBS
> > cruft in the TSO transmit path (nothing like properly updating the
> > driver, eh? No wonder stmmac is such a mess!)
>
> The more I look at this, the more I'm convinced this commit is
> incorrect, even if it is the case that the hardware doesn't support
> TSO and TBS together.
>
> When TSO is enabled (NETIF_F_TSO set in the netif's features) then
> the core net layer can submit skbuffs that need to be processed using
> TSO.
>
> If such a skbuff hits a channel that has TSO disabled (because the
> above commit caused:
>
> stmmac_enable_tso(priv, priv->ioaddr, 1, chan);
>
> not to be called) then the TSE bit in the transmit control register
> will not be set, thereby disabling TSO on this particular channel.
>
> However, stmmac_xmit() will still call through to stmmac_tso_xmit()
> which will dutifully populate the transmit ring with descriptors that
> assume TSE has been set in the transmit control register.
>
> It seems to me _that_ is even more broken than "the hardware doesn't
> support TSO and TBS together" - I have no idea what the stmmac hardware
> does if it encounters descriptors with TSE set but TSE is disabled in
> the transmit control register. My guess would be it would ignore the
> TSE bit in the descriptor and assume that it's one very large packet
> to be sent - and either error out because it's longer than the
> maximum the hardware can support or it will just try to transmit it
> anyway.
Okay, I've found a statement in the stm32mp25xx documentation which
backs up Intel's commit, but that commit is still wrong because it
only half does the job.
However, I think I now have a solution - implementing
.ndo_features_check() which will mask out the NETIF_F_GSO_MASK
features if either the header length is greater than 1023 (the
hardware maximum) or the queue (as returned by
skb_get_queue_mapping(skb)) is for a queue which has TBS available,
and thus has TSO disabled.
I'm surprised we haven't had reports of brokenness in this area.
I think it's time to re-shuffle these patches (plus I think there's
a bit more scope to clean up some of the TSO code which would mean
placing stmmac_set_gso_types() in a different location to keep
all the TSO-related code together. I'll mark this series as
superseded once I have its replacement ready.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* [PATCH] net-shapers: free rollback entries using kfree_rcu
From: Kangzheng Gu @ 2026-03-28 18:58 UTC (permalink / raw)
To: gregkh, davem, edumazet, kuba, pabeni, horms, kees, p,
xiaoguai0992
Cc: netdev, stable, linux-kernel
In-Reply-To: <CAKvcANOzRwFk0jm4xBfMGVNJrgGhBT8zvb6r49qc=WdB5zP_fg@mail.gmail.com>
net_shaper_rollback() removes NET_SHAPER_NOT_VALID entries and frees
them using kfree(), which can race with net_shaper_nl_get_dumpit() and
lead to a use-after-free in net_shaper_fill_one().
Use kfree_rcu() instead of kfree() to free rollback entries, since
net_shaper_nl_get_dumpit() protects shaper access with rcu_read_lock().
Cc: stable@vger.kernel.org
Fixes: 93954b40f6a4 ("net-shapers: implement NL set and delete operations")
Signed-off-by: Kangzheng Gu <xiaoguai0992@gmail.com>
---
net/shaper/shaper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/shaper/shaper.c b/net/shaper/shaper.c
index 94bc9c7382ea..8922f7f64768 100644
--- a/net/shaper/shaper.c
+++ b/net/shaper/shaper.c
@@ -434,7 +434,7 @@ static void net_shaper_rollback(struct net_shaper_binding *binding)
xa_for_each_marked(&hierarchy->shapers, index, cur,
NET_SHAPER_NOT_VALID) {
__xa_erase(&hierarchy->shapers, index);
- kfree(cur);
+ kfree_rcu(cur, rcu);
}
xa_unlock(&hierarchy->shapers);
}
--
2.50.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] net: stmmac: simplify GSO/TSO test in stmmac_xmit()
From: Andrew Lunn @ 2026-03-28 19:01 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Ong Boon Leong, Alexandre Torgue, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, linux-arm-kernel, linux-stm32,
netdev, Paolo Abeni
In-Reply-To: <acgkM2FGS2h-fPVN@shell.armlinux.org.uk>
> I'm surprised we haven't had reports of brokenness in this area.
Could a self test be written for this case?
Andrew
^ permalink raw reply
* [PATCH v3 0/2] stmmac crash/stall fixes when under memory pressure
From: Sam Edwards @ 2026-03-28 19:12 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Maxime Coquelin, Alexandre Torgue, Russell King (Oracle),
Maxime Chevallier, Ovidiu Panait, Vladimir Oltean, Baruch Siach,
Serge Semin, Giuseppe Cavallaro, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, Sam Edwards
Hi netdev,
This is v3 of my series containing a pair of bugfixes for the stmmac driver's
receive pipeline. These issues occur when stmmac_rx_refill() does not (fully)
succeed, which happens more frequently when free memory is low.
The first patch closes Bugzilla bug #221010 [1], where stmmac_rx() can circle
around to a still-dirty descriptor (with a NULL buffer pointer), mistake it for
a filled descriptor (due to OWN=0), and attempt to dereference the buffer.
In testing that patch, I discovered a second issue: starvation of available RX
buffers causes the NIC to stop sending interrupts; if the driver stops polling,
it will wait indefinitely for an interrupt that will never come. (Note: the
first patch makes this issue more prominent -- mostly because it lets the
system survive long enough to exhibit it -- but doesn't *cause* it.) The second
patch addresses that problem as well.
Both patches are minimal, appropriate for stable, and designated to `net`. My
focus is on small, obviously-correct, easy-to-explain changes: I'll follow up
with another patch/series (something like [2]) for `net-next` that fixes the
ring in a more robust way.
The tx and zc paths seem to have similar low-memory bugs, to be addressed in
separate series.
Regards,
Sam
[1] https://bugzilla.kernel.org/show_bug.cgi?id=221010
[2] https://lore.kernel.org/netdev/20260316021009.262358-4-CFSworks@gmail.com/
v3:
- Rebased on latest net/main
- Changed patch 2 to require that stmmac_rx_refill() *fully* succeeds before
exiting polling, to reduce the chance of rx drops.
- DID NOT use the CIRC_SPACE() macro as suggested by Russell: I fear that the
perspective shift (first think of the dirty descriptors as the "work" that
refill "consumes" -- therefore the "space" is how much stmmac_rx() may loop)
is too counterintuitive for a stable fix, but I'll do it in v4 if reviewers
insist.
- Updated the recipients for the series, which was invalidated in v2 due to the
`Fixes:`
v2: https://lore.kernel.org/netdev/20260319184031.8596-1-CFSworks@gmail.com/T/
- Completely rewrote the commit message of patch 1, now assuming the reader is
generally familiar with DMA but wholly unfamiliar with the stmmac device
(thanks Jakub!)
- Added missing `Fixes:` to patch 2
- Moved patch 2's `int budget = limit;` decl per the reverse-xmas-tree rule
- Dropped patch 3: this was a code improvement not appropriate for stable
- Generated the series with --subject-prefix='PATCH net'
v1: https://lore.kernel.org/netdev/20260316021009.262358-1-CFSworks@gmail.com/
Sam Edwards (2):
net: stmmac: Prevent NULL deref when RX memory exhausted
net: stmmac: Prevent indefinite RX stall on buffer exhaustion
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
--
2.52.0
^ permalink raw reply
* [PATCH v3 1/2] net: stmmac: Prevent NULL deref when RX memory exhausted
From: Sam Edwards @ 2026-03-28 19:12 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Maxime Coquelin, Alexandre Torgue, Russell King (Oracle),
Maxime Chevallier, Ovidiu Panait, Vladimir Oltean, Baruch Siach,
Serge Semin, Giuseppe Cavallaro, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, Sam Edwards, stable
In-Reply-To: <20260328191233.519950-1-CFSworks@gmail.com>
The CPU receives frames from the MAC through conventional DMA: the CPU
allocates buffers for the MAC, then the MAC fills them and returns
ownership to the CPU. For each hardware RX queue, the CPU and MAC
coordinate through a shared ring array of DMA descriptors: one
descriptor per DMA buffer. Each descriptor includes the buffer's
physical address and a status flag ("OWN") indicating which side owns
the buffer: OWN=0 for CPU, OWN=1 for MAC. The CPU is only allowed to set
the flag and the MAC is only allowed to clear it, and both must move
through the ring in sequence: thus the ring is used for both
"submissions" and "completions."
In the stmmac driver, stmmac_rx() bookmarks its position in the ring
with the `cur_rx` index. The main receive loop in that function checks
for rx_descs[cur_rx].own=0, gives the corresponding buffer to the
network stack (NULLing the pointer), and increments `cur_rx` modulo the
ring size. After the loop exits, stmmac_rx_refill(), which bookmarks its
position with `dirty_rx`, allocates fresh buffers and rearms the
descriptors (setting OWN=1). If it fails any allocation, it simply stops
early (leaving OWN=0) and will retry where it left off when next called.
This means descriptors have a three-stage lifecycle (terms my own):
- `empty` (OWN=1, buffer valid)
- `full` (OWN=0, buffer valid and populated)
- `dirty` (OWN=0, buffer NULL)
But because stmmac_rx() only checks OWN, it confuses `full`/`dirty`. In
the past (see 'Fixes:'), there was a bug where the loop could cycle
`cur_rx` all the way back to the first descriptor it dirtied, resulting
in a NULL dereference when mistaken for `full`. The aforementioned
commit resolved that *specific* failure by capping the loop's iteration
limit at `dma_rx_size - 1`, but this is only a partial fix: if the
previous stmmac_rx_refill() didn't complete, then there are leftover
`dirty` descriptors that the loop might encounter without needing to
cycle fully around. The current code therefore panics (see 'Closes:')
when stmmac_rx_refill() is memory-starved long enough for `cur_rx` to
catch up to `dirty_rx`.
Fix this by further tightening the clamp from `dma_rx_size - 1` to
`dma_rx_size - stmmac_rx_dirty() - 1`, subtracting any remnant dirty
entries and limiting the loop so that `cur_rx` cannot catch back up to
`dirty_rx`. This carries no risk of arithmetic underflow: since the
maximum possible return value of stmmac_rx_dirty() is `dma_rx_size - 1`,
the worst the clamp can do is prevent the loop from running at all.
Fixes: b6cb4541853c7 ("net: stmmac: avoid rx queue overrun")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221010
Cc: stable@vger.kernel.org
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6827c99bde8c..f98b070073c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5609,7 +5609,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
dma_dir = page_pool_get_dma_dir(rx_q->page_pool);
bufsz = DIV_ROUND_UP(priv->dma_conf.dma_buf_sz, PAGE_SIZE) * PAGE_SIZE;
- limit = min(priv->dma_conf.dma_rx_size - 1, (unsigned int)limit);
+ limit = min(priv->dma_conf.dma_rx_size - stmmac_rx_dirty(priv, queue) - 1,
+ (unsigned int)limit);
if (netif_msg_rx_status(priv)) {
void *rx_head;
--
2.52.0
^ permalink raw reply related
* [PATCH v3 2/2] net: stmmac: Prevent indefinite RX stall on buffer exhaustion
From: Sam Edwards @ 2026-03-28 19:12 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Maxime Coquelin, Alexandre Torgue, Russell King (Oracle),
Maxime Chevallier, Ovidiu Panait, Vladimir Oltean, Baruch Siach,
Serge Semin, Giuseppe Cavallaro, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, Sam Edwards, stable
In-Reply-To: <20260328191233.519950-1-CFSworks@gmail.com>
The stmmac driver handles interrupts in the usual NAPI way: an interrupt
arrives, the NAPI instance is scheduled and interrupts are masked, and
the actual work occurs in the NAPI polling function. Once no further
work remains, interrupts are unmasked and the NAPI instance is put to
sleep to await a future interrupt. In the receive case, the MAC only
sends the interrupt when a DMA operation completes; thus the driver must
make sure a usable RX DMA descriptor exists before expecting a future
interrupt.
The main receive loop in stmmac_rx() exits under one of 3 conditions:
1) It encounters a DMA descriptor with OWN=1, indicating that no further
pending data exists. The MAC will use this descriptor for the next
RX DMA operation, so the driver can expect a future interrupt.
2) It exhausts the NAPI budget. In this case, the driver doesn't know
whether the MAC has any usable DMA descriptors. But when the driver
consumes its full budget, that signals NAPI to keep polling, so the
question is moot.
3) It runs out of (non-dirty) descriptors in the RX ring. In this case,
the MAC will only have a usable descriptor if stmmac_rx_refill()
succeeds (at least partially).
Currently, stmmac_rx() lacks any check against scenario #3 and
stmmac_rx_refill() failing: it will stop NAPI polling and unmask
interrupts to await an interrupt that will never arrive, stalling the
receive pipeline indefinitely.
Fix this by checking stmmac_rx_dirty(): it will return 0 if
stmmac_rx_refill() fully succeeded and we can safely await an interrupt.
Any nonzero value means some allocations failed, in which case we risk
dropping frames if a large traffic burst exhausts the surviving
non-dirties. Therefore, simply return the full budget (to keep polling)
until all allocations succeed.
Fixes: 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
Cc: stable@vger.kernel.org
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f98b070073c0..81f764352f3d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5604,6 +5604,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
unsigned int desc_size;
struct sk_buff *skb = NULL;
struct stmmac_xdp_buff ctx;
+ int budget = limit;
int xdp_status = 0;
int bufsz;
@@ -5870,6 +5871,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
priv->xstats.rx_dropped += rx_dropped;
priv->xstats.rx_errors += rx_errors;
+ /* If stmmac_rx_refill() failed, keep trying until it doesn't. */
+ if (unlikely(stmmac_rx_dirty(priv, queue) > 0))
+ return budget;
+
return count;
}
--
2.52.0
^ permalink raw reply related
* [PATCH net] net/mlx5: fs, fix invalid pointer dereference in mlx5_fs_add_rule tracepoint
From: kenneth @ 2026-03-28 19:20 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Yevgeny Kliteynik, Moshe Shemesh, netdev, linux-rdma,
linux-kernel, Kenneth Klette Jonassen, stable
From: Kenneth Klette Jonassen <kenneth@bridgetech.tv>
The mlx5_fs_add_rule tracepoint has used the flow destination type in
a bitwise test since its introduction. However, that's not a valid way
to treat it anymore (if it ever was), and after commit d639af621600dc
("net/mlx5: fs, split software and IFC flow destination definitions"),
this mismatch caused nearly any destination type to be mistaken as a
flow counter, and thus stashing 32 bits of the mlx5_flow_destination
union into the counter_id field of the tracepoint.
Later commit 95f68e06b41b9e ("net/mlx5: fs, add counter object to flow
destination") exacerbates this issue by converting the counter union
member from an integer to a pointer. Now the tracepoint dereferences
whichever value is in the union, and in cases where that's not a valid
pointer, it can lead to a kernel oops.
Fix the check. Reported by GitHub user whi71800.
Cc: stable@vger.kernel.org
Fixes: 95f68e06b41b9e ("net/mlx5: fs, add counter object to flow destination")
Closes: https://github.com/knneth/mlnx-ofa_kernel/issues/1
Signed-off-by: Kenneth Klette Jonassen <kenneth@bridgetech.tv>
---
drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.h b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.h
index d6e736c1fb24..b099fe71b781 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fs_tracepoint.h
@@ -289,7 +289,7 @@ TRACE_EVENT(mlx5_fs_add_rule,
memcpy(__entry->destination,
&rule->dest_attr,
sizeof(__entry->destination));
- if (rule->dest_attr.type &
+ if (rule->dest_attr.type ==
MLX5_FLOW_DESTINATION_TYPE_COUNTER)
__entry->counter_id =
mlx5_fc_id(rule->dest_attr.counter);
--
2.43.0
^ permalink raw reply related
* [RESEND PATCH net v3 0/2] stmmac crash/stall fixes when under memory pressure
From: Sam Edwards @ 2026-03-28 19:25 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Maxime Coquelin, Alexandre Torgue, Russell King (Oracle),
Maxime Chevallier, Ovidiu Panait, Vladimir Oltean, Baruch Siach,
Serge Semin, Giuseppe Cavallaro, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, Sam Edwards
Hi netdev,
This is v3 of my series containing a pair of bugfixes for the stmmac driver's
receive pipeline. These issues occur when stmmac_rx_refill() does not (fully)
succeed, which happens more frequently when free memory is low.
The first patch closes Bugzilla bug #221010 [1], where stmmac_rx() can circle
around to a still-dirty descriptor (with a NULL buffer pointer), mistake it for
a filled descriptor (due to OWN=0), and attempt to dereference the buffer.
In testing that patch, I discovered a second issue: starvation of available RX
buffers causes the NIC to stop sending interrupts; if the driver stops polling,
it will wait indefinitely for an interrupt that will never come. (Note: the
first patch makes this issue more prominent -- mostly because it lets the
system survive long enough to exhibit it -- but doesn't *cause* it.) The second
patch addresses that problem as well.
Both patches are minimal, appropriate for stable, and designated to `net`. My
focus is on small, obviously-correct, easy-to-explain changes: I'll follow up
with another patch/series (something like [2]) for `net-next` that fixes the
ring in a more robust way.
The tx and zc paths seem to have similar low-memory bugs, to be addressed in
separate series.
Regards,
Sam
---
RESEND: Oops, my subject line didn't have `net`, so I'm resending the series to
fix that.
[1] https://bugzilla.kernel.org/show_bug.cgi?id=221010
[2] https://lore.kernel.org/netdev/20260316021009.262358-4-CFSworks@gmail.com/
v3:
- Rebased on latest net/main
- Changed patch 2 to require that stmmac_rx_refill() *fully* succeeds before
exiting polling, to reduce the chance of rx drops.
- DID NOT use the CIRC_SPACE() macro as suggested by Russell: I fear that the
perspective shift (first think of the dirty descriptors as the "work" that
refill "consumes" -- therefore the "space" is how much stmmac_rx() may loop)
is too counterintuitive for a stable fix, but I'll do it in v4 if reviewers
insist.
- Updated the recipients for the series, which was invalidated in v2 due to the
`Fixes:`
v2: https://lore.kernel.org/netdev/20260319184031.8596-1-CFSworks@gmail.com/T/
- Completely rewrote the commit message of patch 1, now assuming the reader is
generally familiar with DMA but wholly unfamiliar with the stmmac device
(thanks Jakub!)
- Added missing `Fixes:` to patch 2
- Moved patch 2's `int budget = limit;` decl per the reverse-xmas-tree rule
- Dropped patch 3: this was a code improvement not appropriate for stable
- Generated the series with --subject-prefix='PATCH net'
v1: https://lore.kernel.org/netdev/20260316021009.262358-1-CFSworks@gmail.com/
Sam Edwards (2):
net: stmmac: Prevent NULL deref when RX memory exhausted
net: stmmac: Prevent indefinite RX stall on buffer exhaustion
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
--
2.52.0
^ permalink raw reply
* [RESEND PATCH net v3 1/2] net: stmmac: Prevent NULL deref when RX memory exhausted
From: Sam Edwards @ 2026-03-28 19:25 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Maxime Coquelin, Alexandre Torgue, Russell King (Oracle),
Maxime Chevallier, Ovidiu Panait, Vladimir Oltean, Baruch Siach,
Serge Semin, Giuseppe Cavallaro, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, Sam Edwards, stable
In-Reply-To: <20260328192503.520689-1-CFSworks@gmail.com>
The CPU receives frames from the MAC through conventional DMA: the CPU
allocates buffers for the MAC, then the MAC fills them and returns
ownership to the CPU. For each hardware RX queue, the CPU and MAC
coordinate through a shared ring array of DMA descriptors: one
descriptor per DMA buffer. Each descriptor includes the buffer's
physical address and a status flag ("OWN") indicating which side owns
the buffer: OWN=0 for CPU, OWN=1 for MAC. The CPU is only allowed to set
the flag and the MAC is only allowed to clear it, and both must move
through the ring in sequence: thus the ring is used for both
"submissions" and "completions."
In the stmmac driver, stmmac_rx() bookmarks its position in the ring
with the `cur_rx` index. The main receive loop in that function checks
for rx_descs[cur_rx].own=0, gives the corresponding buffer to the
network stack (NULLing the pointer), and increments `cur_rx` modulo the
ring size. After the loop exits, stmmac_rx_refill(), which bookmarks its
position with `dirty_rx`, allocates fresh buffers and rearms the
descriptors (setting OWN=1). If it fails any allocation, it simply stops
early (leaving OWN=0) and will retry where it left off when next called.
This means descriptors have a three-stage lifecycle (terms my own):
- `empty` (OWN=1, buffer valid)
- `full` (OWN=0, buffer valid and populated)
- `dirty` (OWN=0, buffer NULL)
But because stmmac_rx() only checks OWN, it confuses `full`/`dirty`. In
the past (see 'Fixes:'), there was a bug where the loop could cycle
`cur_rx` all the way back to the first descriptor it dirtied, resulting
in a NULL dereference when mistaken for `full`. The aforementioned
commit resolved that *specific* failure by capping the loop's iteration
limit at `dma_rx_size - 1`, but this is only a partial fix: if the
previous stmmac_rx_refill() didn't complete, then there are leftover
`dirty` descriptors that the loop might encounter without needing to
cycle fully around. The current code therefore panics (see 'Closes:')
when stmmac_rx_refill() is memory-starved long enough for `cur_rx` to
catch up to `dirty_rx`.
Fix this by further tightening the clamp from `dma_rx_size - 1` to
`dma_rx_size - stmmac_rx_dirty() - 1`, subtracting any remnant dirty
entries and limiting the loop so that `cur_rx` cannot catch back up to
`dirty_rx`. This carries no risk of arithmetic underflow: since the
maximum possible return value of stmmac_rx_dirty() is `dma_rx_size - 1`,
the worst the clamp can do is prevent the loop from running at all.
Fixes: b6cb4541853c7 ("net: stmmac: avoid rx queue overrun")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221010
Cc: stable@vger.kernel.org
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6827c99bde8c..f98b070073c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5609,7 +5609,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
dma_dir = page_pool_get_dma_dir(rx_q->page_pool);
bufsz = DIV_ROUND_UP(priv->dma_conf.dma_buf_sz, PAGE_SIZE) * PAGE_SIZE;
- limit = min(priv->dma_conf.dma_rx_size - 1, (unsigned int)limit);
+ limit = min(priv->dma_conf.dma_rx_size - stmmac_rx_dirty(priv, queue) - 1,
+ (unsigned int)limit);
if (netif_msg_rx_status(priv)) {
void *rx_head;
--
2.52.0
^ permalink raw reply related
* [RESEND PATCH net v3 2/2] net: stmmac: Prevent indefinite RX stall on buffer exhaustion
From: Sam Edwards @ 2026-03-28 19:25 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Maxime Coquelin, Alexandre Torgue, Russell King (Oracle),
Maxime Chevallier, Ovidiu Panait, Vladimir Oltean, Baruch Siach,
Serge Semin, Giuseppe Cavallaro, netdev, linux-stm32,
linux-arm-kernel, linux-kernel, Sam Edwards, stable
In-Reply-To: <20260328192503.520689-1-CFSworks@gmail.com>
The stmmac driver handles interrupts in the usual NAPI way: an interrupt
arrives, the NAPI instance is scheduled and interrupts are masked, and
the actual work occurs in the NAPI polling function. Once no further
work remains, interrupts are unmasked and the NAPI instance is put to
sleep to await a future interrupt. In the receive case, the MAC only
sends the interrupt when a DMA operation completes; thus the driver must
make sure a usable RX DMA descriptor exists before expecting a future
interrupt.
The main receive loop in stmmac_rx() exits under one of 3 conditions:
1) It encounters a DMA descriptor with OWN=1, indicating that no further
pending data exists. The MAC will use this descriptor for the next
RX DMA operation, so the driver can expect a future interrupt.
2) It exhausts the NAPI budget. In this case, the driver doesn't know
whether the MAC has any usable DMA descriptors. But when the driver
consumes its full budget, that signals NAPI to keep polling, so the
question is moot.
3) It runs out of (non-dirty) descriptors in the RX ring. In this case,
the MAC will only have a usable descriptor if stmmac_rx_refill()
succeeds (at least partially).
Currently, stmmac_rx() lacks any check against scenario #3 and
stmmac_rx_refill() failing: it will stop NAPI polling and unmask
interrupts to await an interrupt that will never arrive, stalling the
receive pipeline indefinitely.
Fix this by checking stmmac_rx_dirty(): it will return 0 if
stmmac_rx_refill() fully succeeded and we can safely await an interrupt.
Any nonzero value means some allocations failed, in which case we risk
dropping frames if a large traffic burst exhausts the surviving
non-dirties. Therefore, simply return the full budget (to keep polling)
until all allocations succeed.
Fixes: 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
Cc: stable@vger.kernel.org
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f98b070073c0..81f764352f3d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5604,6 +5604,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
unsigned int desc_size;
struct sk_buff *skb = NULL;
struct stmmac_xdp_buff ctx;
+ int budget = limit;
int xdp_status = 0;
int bufsz;
@@ -5870,6 +5871,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
priv->xstats.rx_dropped += rx_dropped;
priv->xstats.rx_errors += rx_errors;
+ /* If stmmac_rx_refill() failed, keep trying until it doesn't. */
+ if (unlikely(stmmac_rx_dirty(priv, queue) > 0))
+ return budget;
+
return count;
}
--
2.52.0
^ permalink raw reply related
* Re: [PATCH net-next v2] dt-bindings: net: wireless: brcm: Add compatible for bcm43752
From: Arend van Spriel @ 2026-03-28 19:48 UTC (permalink / raw)
To: Ronald Claveau, Johannes Berg, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, van Spriel
Cc: linux-wireless, devicetree, linux-kernel, netdev, Conor Dooley
In-Reply-To: <20260327-add-bcm43752-compatible-v2-1-5b28e6637101@aliel.fr>
On 27/03/2026 10:36, Ronald Claveau wrote:
> Add bcm43752 compatible with its bcm4329 compatible fallback.
Looks pretty trivial so no remarks from me here.
Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> ---
> The Khadas VIM4 board based on Amlogic A311D2 aka T7 features an AP6275s Wi-Fi/Bluetooth module with a BCM43752 chipset.
> This patch aims to add this chipset with its fallback to bcm4329 compatible.
>
> The original patch series is here:
> https://lore.kernel.org/r/20260326-add-emmc-t7-vim4-v5-0-d3f182b48e9d@aliel.fr
> ---
> Changes in v2:
> - Add netdev in CC.
> - Link to v1: https://lore.kernel.org/r/20260326-add-bcm43752-compatible-v1-1-b3b9a58ab38b@aliel.fr
> ---
> Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml | 1 +
> 1 file changed, 1 insertion(+)
^ permalink raw reply
* AW: [PATCH net-next] net: phy: realtek: support MDI swapping for RTL8226-CG
From: markus.stockhausen @ 2026-03-28 20:02 UTC (permalink / raw)
To: 'Jan Hoffmann', 'Andrew Lunn',
'Heiner Kallweit', 'Russell King',
'David S. Miller', 'Eric Dumazet',
'Jakub Kicinski', 'Paolo Abeni',
'Daniel Golle', 'Damien Dejean'
Cc: netdev, linux-kernel
In-Reply-To: <20260322193235.990881-1-jan@3e8.eu>
> Von: Jan Hoffmann <jan@3e8.eu>
> Betreff: [PATCH net-next] net: phy: realtek: support MDI swapping for
RTL8226-CG
>
> Add support for configuring swapping of MDI pairs (ABCD->DCBA) when the
> property "enet-phy-pair-order" is specified.
> ...
> ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2, 0xd068, 0x7, 0x1);
No real knowledge here. But from other register combinations around
the Realtek ecosystem this could make sense:
#define RTL822X_MDI_CAL_CTRL 0xd068 /* MDI pair cal index/control */
#define RTL822X_MDI_CAL_DATA 0xd06a /* MDI pair cal data */
Markus
^ permalink raw reply
* Re: [PATCH net-next] net: sfp: add quirk for ZOERAX SFP-2.5G-T
From: Jan Hoffmann @ 2026-03-28 20:02 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Russell King, Andrew Lunn, Heiner Kallweit, David S. Miller,
Eric Dumazet, Paolo Abeni, netdev, linux-kernel
In-Reply-To: <20260327205020.1c020731@kernel.org>
On 2026-03-28 at 04:50, Jakub Kicinski wrote:
> On Wed, 25 Mar 2026 17:35:58 +0100 Jan Hoffmann wrote:
>> This is a 2.5G copper module which appears to be based on a Motorcomm
>> YT8821 PHY. There doesn't seem to be a usable way to to access the PHY
>> (I2C address 0x56 provides only read-only C22 access, and Rollball is
>> also not working).
>>
>> The module does not report the correct extended compliance code for
>> 2.5GBase-T, and instead claims to support SONET OC-48 and Fibre Channel:
>>
>> Identifier : 0x03 (SFP)
>> Extended identifier : 0x04 (GBIC/SFP defined by 2-wire interface ID)
>> Connector : 0x07 (LC)
>> Transceiver codes : 0x00 0x01 0x00 0x00 0x40 0x40 0x04 0x00 0x00
>> Transceiver type : FC: Multimode, 50um (M5)
>> Encoding : 0x05 (SONET Scrambled)
>> BR Nominal : 2500MBd
>>
>> Despite this, the kernel still sets 2500Base-X as interface mode based
>> on the (incorrect) nominal signaling rate.
>>
>> However, it is also necessary to disable auto-negotiation for the module
>> to actually work. Thus, create a SFP quirk to do this.
>>
>> Signed-off-by: Jan Hoffmann <jan@3e8.eu>
>> ---
>> Note: I'm not quite sure "sfp_quirk_disable_autoneg" is enough here, or
>> if it would be appropriate to use the full "sfp_quirk_oem_2_5g" quirk
>> due to the not really correct compliance code / nominal signaling rate.
>
> I'm no SFP expert but just going by the naming I strongly suspect that
> the module is the same hardware as "SFP-2.5G-T" from "FS", so keeping
> quirks consistent makes sense?
Based on commit e27aca3760c0 ("net: sfp: enhance quirk for Fibrestore
2.5G copper SFP module"), the FS module uses an RTL8221B PHY, so it is
different hardware. And it also supports the Rollball protocol for PHY
access, unlike this Zoerax module.
From the existing quirks, the only valid alternative I see for the
Zoerax module would be "sfp_quirk_oem_2_5g" (which also sets the
additional bits for 2500Base-T link mode and explicitly enables
2500Base-X interface mode). But as that is not strictly necessary for
making this particular module work, I am unsure if using this quirk
would be appropriate.
Thanks,
Jan
^ permalink raw reply
* Re: [PATCH net-next 0/5] veth: add Byte Queue Limits (BQL) support
From: Toke Høiland-Jørgensen @ 2026-03-28 20:06 UTC (permalink / raw)
To: Jonas Köppeler, Jesper Dangaard Brouer, netdev
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, jhs, jiri,
kernel-team, Chris Arges, Mike Freemon
In-Reply-To: <7d404bd3-4444-464e-8831-c8304ecf5b40@tu-berlin.de>
Jonas Köppeler <j.koeppeler@tu-berlin.de> writes:
> On 3/27/26 13:49, Jesper Dangaard Brouer wrote:
>>
>>
>> On 27/03/2026 10.50, Toke Høiland-Jørgensen wrote:
>>> hawk@kernel.org writes:
>>>
>>>> From: Jesper Dangaard Brouer <hawk@kernel.org>
>>>>
>>>> This series adds BQL (Byte Queue Limits) to the veth driver, reducing
>>>> latency by dynamically limiting in-flight bytes in the ptr_ring and
>>>> moving buffering into the qdisc where AQM algorithms can act on it.
>>>>
>>>> Problem:
>>>> veth's 256-entry ptr_ring acts as a "dark buffer" -- packets queued
>>>> there are invisible to the qdisc's AQM. Under load, the ring fills
>>>> completely (DRV_XOFF backpressure), adding up to 256 packets of
>>>> unmanaged latency before the qdisc even sees congestion.
>>>>
>>>> Solution:
>>>> BQL (STACK_XOFF) dynamically limits in-flight bytes, stopping the
>>>> queue before the ring fills. This keeps the ring shallow and pushes
>>>> excess packets into the qdisc, where sojourn-based AQM can measure
>>>> and drop them.
>>>
>>> So one question here: Is *Byte* queue limits really the right thing for
>>> veth? As you mention above, the ptr_ring is sized in a number of
>>> packets. On a physical NIC, accounting bytes makes sense because there's
>>> a fixed line rate, so bytes turn directly into latency.
>>>
>>> But on a veth device, the stack processing is per packet, and most
>>> processing takes the same amount of time regardless of the size of the
>>> packet (e.g., netfilter rules that operate on the skb only).
>>>
>>> So my worry would be that when you're accounting in bytes, if there's a
>>> mix of big and small packets, you'd end up with the BQL algorithm
>>> scaling to a "too large" value, which would allow a lot of small packets
>>> to be queued up, adding extra latency (or even overflowing the ring
>>> buffer if the ratio is large enough).
>>>
>>> Have you run any such experiments?
>>
>> Thank for bring this up.
>> Yes, we have considered this (and agree).
>>
>> Jonas is conduction some experiments.
>> I will let Jonas answer?
> Hi,
>
> I used the provided selftest, modified so that the payload size alternates
> between 1400 bytes and sizeof(struct pkt_hdr) = 24 bytes every 5000 packets.
>
> The receiver was slowed down using 10K iptables rules. I could confirm that
> the receive queue filled up to ~66 packets, whereas the BQL limit is around
> 2884 bytes, corresponding to approximately 2 x 1400-byte packets.
>
> I compared two accounting strategies: using skb->len vs. a fixed size of 1.
>
> Ping results over 5 runs using skb->len accounting:
>
> rtt min/avg/max/mdev = 0.636/2.784/ 9.543/1.735 ms
> rtt min/avg/max/mdev = 0.629/2.947/10.587/1.927 ms
> rtt min/avg/max/mdev = 0.587/2.966/11.625/1.963 ms
> rtt min/avg/max/mdev = 0.589/3.006/10.694/1.979 ms
>
> Ping results over 5 runs using fixed size (1) accounting:
>
> rtt min/avg/max/mdev = 0.587/2.446/6.261/1.065 ms
> rtt min/avg/max/mdev = 0.641/2.339/6.008/0.950 ms
> rtt min/avg/max/mdev = 0.688/2.527/5.506/1.086 ms
> rtt min/avg/max/mdev = 0.596/2.411/5.228/1.041 ms
>
> The avg and max RTT are consistently lower with the fixed-size accounting.
> This suggests that the excess buffered packets contribute to some
> latency.
Right, so this sounds like fixed-size accounting is the way to go, then.
Cool :)
-Toke
^ permalink raw reply
* Re: [PATCH net-next v4 4/6] net: bcmgenet: add XDP_TX support
From: Nicolai Buchwitz @ 2026-03-28 20:36 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, Justin Chen, Simon Horman, Doug Berger, Florian Fainelli,
Broadcom internal kernel review list, Andrew Lunn,
David S. Miller, Eric Dumazet, Paolo Abeni, Alexei Starovoitov,
Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
Stanislav Fomichev, linux-kernel, bpf
In-Reply-To: <20260324210342.61b0b5f8@kernel.org>
On 25.3.2026 05:03, Jakub Kicinski wrote:
> On Mon, 23 Mar 2026 13:05:33 +0100 Nicolai Buchwitz wrote:
>> Implement XDP_TX using ring 16 (DESC_INDEX), the hardware default
>> descriptor ring, dedicated to XDP TX for isolation from SKB TX queues.
>>
>> Ring 16 gets 32 BDs carved from ring 0's allocation. TX completion is
>> piggybacked on RX NAPI poll since ring 16's INTRL2_1 bit collides with
>> RX ring 0, similar to how bnxt, ice, and other XDP drivers handle TX
>> completion within the RX poll path.
>>
>> The GENET MAC has TBUF_64B_EN set globally, requiring every TX buffer
>> to start with a 64-byte struct status_64 (TSB). For local XDP_TX, the
>> TSB is prepended by backing xdp->data into the RSB area (unused after
>> BPF execution) and zeroing it. For foreign frames redirected from
>> other
>> devices, the TSB is written into the xdp_frame headroom.
>>
>> The page_pool DMA direction is changed from DMA_FROM_DEVICE to
>> DMA_BIDIRECTIONAL to allow TX reuse of the existing DMA mapping.
>
> drivers/net/ethernet/broadcom/genet/bcmgenet.c:2420:27: warning:
> variable 'tx_ring' set but not used [-Wunused-but-set-variable]
> 2420 | struct bcmgenet_tx_ring *tx_ring;
> | ^
Missed during the refactoring (should be part of patch 5). I will
address this in the next version of this series.
Thanks
Nicolai
^ permalink raw reply
* [PATCH] net: sched: cls_api: fix tc_chain_fill_node to initialize tcm_info to zero to prevent an info-leak
From: Yochai Eisenrich @ 2026-03-28 21:14 UTC (permalink / raw)
To: David S . Miller
Cc: Yochai Eisenrich, Jamal Hadi Salim, Jiri Pirko, security, netdev
When building netlink messages, tc_chain_fill_node() never initializes
the tcm_info field of struct tcmsg. Since the allocation is not zeroed,
kernel heap memory is leaked to userspace through this 4-byte field.
The fix simply zeroes tcm_info alongside the other fields that are
already initialized.
Fixes: 32a4f5ecd738 ("net: sched: introduce chain object to uapi")
Signed-off-by: Yochai Eisenrich <echelonh@gmail.com>
---
net/sched/cls_api.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 4829c27446e3..20f7f9ee0b35 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2969,6 +2969,7 @@ static int tc_chain_fill_node(const struct tcf_proto_ops *tmplt_ops,
tcm->tcm__pad1 = 0;
tcm->tcm__pad2 = 0;
tcm->tcm_handle = 0;
+ tcm->tcm_info = 0;
if (block->q) {
tcm->tcm_ifindex = qdisc_dev(block->q)->ifindex;
tcm->tcm_parent = block->q->handle;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH net v3] rtnetlink: add missing netlink_ns_capable() check for peer netns
From: Kuniyuki Iwashima @ 2026-03-28 21:18 UTC (permalink / raw)
To: Nikolaos Gkarlis; +Cc: netdev, kuba
In-Reply-To: <20260328135215.901879-1-nickgarlis@gmail.com>
On Sat, Mar 28, 2026 at 6:52 AM Nikolaos Gkarlis <nickgarlis@gmail.com> wrote:
>
> rtnl_newlink() lacks a CAP_NET_ADMIN capability check on the peer
> network namespace when creating paired devices (veth, vxcan,
> netkit). This allows an unprivileged user with a user namespace
> to create interfaces in arbitrary network namespaces, including
> init_net.
>
> Add a netlink_ns_capable() check for CAP_NET_ADMIN in the peer
> namespace before allowing device creation to proceed.
>
> Fixes: 81adee47dfb6 ("net: Support specifying the network namespace upon device creation.")
> Signed-off-by: Nikolaos Gkarlis <nickgarlis@gmail.com>
> ---
> v3:
> - Move netlink_ns_capable() check from rtnl_newlink() into
> rtnl_get_peer_net(), after the last rtnl_link_get_net_ifla(tb)
> call. The tbp path is already covered by rtnl_link_get_net_capable()
> in the caller. (suggested by Kuniyuki)
> - Pass skb to rtnl_get_peer_net() for the capability check.
> - Add IS_ERR() check on rtnl_link_get_net_ifla(tb) return value.
> v2:
> - Removed "Reported-by" tag
> - Fixed "Fixes" tag with the help of Kuniyuki Iwashima (thanks !)
>
> net/core/rtnetlink.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index fae8034efbf..4be7fc6e23a 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -3894,12 +3894,14 @@ static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm,
> goto out;
> }
>
> -static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
> +static struct net *rtnl_get_peer_net(struct sk_buff *skb,
> + const struct rtnl_link_ops *ops,
> struct nlattr *tbp[],
> struct nlattr *data[],
> struct netlink_ext_ack *extack)
> {
> struct nlattr *tb[IFLA_MAX + 1];
> + struct net *net;
> int err;
>
> if (!data || !data[ops->peer_type])
> @@ -3915,7 +3917,19 @@ static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
> return ERR_PTR(err);
> }
>
> - return rtnl_link_get_net_ifla(tb);
> + net = rtnl_link_get_net_ifla(tb);
> + if (IS_ERR(net))
nit: use IS_ERR_OR_NULL and remove "if (!net) below
> + return net;
> +
> + if (!net)
> + return NULL;
> +
> + if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
> + put_net(net);
> + return ERR_PTR(-EPERM);
> + }
> +
> + return net;
> }
>
> static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> @@ -4054,7 +4068,7 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
> }
>
> if (ops->peer_type) {
> - peer_net = rtnl_get_peer_net(ops, tb, data, extack);
> + peer_net = rtnl_get_peer_net(skb, ops, tb, data, extack);
> if (IS_ERR(peer_net)) {
> ret = PTR_ERR(peer_net);
> goto put_ops;
> --
> 2.34.1
>
^ permalink raw reply
* [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
From: Nikolaos Gkarlis @ 2026-03-28 21:33 UTC (permalink / raw)
To: netdev; +Cc: kuba, nickgarlis, kuniyu
rtnl_newlink() lacks a CAP_NET_ADMIN capability check on the peer
network namespace when creating paired devices (veth, vxcan,
netkit). This allows an unprivileged user with a user namespace
to create interfaces in arbitrary network namespaces, including
init_net.
Add a netlink_ns_capable() check for CAP_NET_ADMIN in the peer
namespace before allowing device creation to proceed.
Fixes: 81adee47dfb6 ("net: Support specifying the network namespace upon device creation.")
Signed-off-by: Nikolaos Gkarlis <nickgarlis@gmail.com>
---
v4:
- Use IS_ERR_OR_NULL instead of IS_ERR + null check.
v3:
- Move netlink_ns_capable() check from rtnl_newlink() into
rtnl_get_peer_net(), after the last rtnl_link_get_net_ifla(tb)
call. The tbp path is already covered by rtnl_link_get_net_capable()
in the caller. (suggested by Kuniyuki)
- Pass skb to rtnl_get_peer_net() for the capability check.
- Add IS_ERR() check on rtnl_link_get_net_ifla(tb) return value.
v2:
- Removed "Reported-by" tag
- Fixed "Fixes" tag with the help of Kuniyuki Iwashima (thanks !)
net/core/rtnetlink.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index fae8034efbf..a4d8fd8232e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3894,12 +3894,14 @@ static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm,
goto out;
}
-static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
+static struct net *rtnl_get_peer_net(struct sk_buff *skb,
+ const struct rtnl_link_ops *ops,
struct nlattr *tbp[],
struct nlattr *data[],
struct netlink_ext_ack *extack)
{
struct nlattr *tb[IFLA_MAX + 1];
+ struct net *net;
int err;
if (!data || !data[ops->peer_type])
@@ -3915,7 +3917,16 @@ static struct net *rtnl_get_peer_net(const struct rtnl_link_ops *ops,
return ERR_PTR(err);
}
- return rtnl_link_get_net_ifla(tb);
+ net = rtnl_link_get_net_ifla(tb);
+ if (IS_ERR_OR_NULL(net))
+ return net;
+
+ if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
+ put_net(net);
+ return ERR_PTR(-EPERM);
+ }
+
+ return net;
}
static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -4054,7 +4065,7 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
}
if (ops->peer_type) {
- peer_net = rtnl_get_peer_net(ops, tb, data, extack);
+ peer_net = rtnl_get_peer_net(skb, ops, tb, data, extack);
if (IS_ERR(peer_net)) {
ret = PTR_ERR(peer_net);
goto put_ops;
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox