* Re: [net-next PATCH 06/10] net: dsa: realtek: rtl8365mb: add VLAN support
From: Luiz Angelo Daros de Luca @ 2026-04-02 2:45 UTC (permalink / raw)
To: Yury Norov
Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Linus Walleij,
Alvin Šipraga, Yury Norov, Rasmus Villemoes, Russell King,
netdev, linux-kernel
In-Reply-To: <acyCmHh1IAykwycy@yury>
> > + vlan4k->vid = vid;
> > + vlan4k->member =
> > + FIELD_GET(RTL8365MB_CVLAN_ENTRY_D0_MBR_MASK, data[0]) |
> > + (FIELD_GET(RTL8365MB_CVLAN_ENTRY_D2_MBR_EXT_MASK, data[2])
> > + << FIELD_WIDTH(RTL8365MB_CVLAN_ENTRY_D0_MBR_MASK));
>
> This FIELD_GET() << FIELD_WIDTH() resembles FIELD_PREP(), except that
> you skip some checks. Is that intentional?
While it resembles FIELD_PREP(), the usage of FIELD_GET() <<
FIELD_WIDTH() is intentional and semantically distinct from
FIELD_PREP(). While FIELD_PREP() prepares a raw value to be written up
into a register mask, we are performing a read/unpack operation here:
extracting bits from hardware registers to populate the driver's
internal logical structure.
Using FIELD_PREP() for this would require defining a 'fake' software
mask (e.g., GENMASK(10, 8)) that does not correspond to any physical
register, which would be more confusing to future maintainers than the
current explicit reassembly.
The 11-bit member field is split across two non-contiguous 16-bit
words in the CVLAN entry:
- data[0] (D0): Contains the primary bits [7:0] via
RTL8365MB_CVLAN_ENTRY_D0_MBR_MASK.
- data[2] (D2): Contains the extension bits [10:8] via
RTL8365MB_CVLAN_ENTRY_D2_MBR_EXT_MASK.
This split layout suggests the hardware was originally designed for 8
ports and later expanded to 11, placing the extra bits in the
previously unused space in data[2].
Even if we concatenate them into a single 48-bit value, a single
bitfield macro would not handle the non-contiguous mask.
Regards,
Luiz
^ permalink raw reply
* Re: [PATCH net v4] rtnetlink: add missing netlink_ns_capable() check for peer netns
From: Jakub Kicinski @ 2026-04-02 2:45 UTC (permalink / raw)
To: Nikolaos Gkarlis; +Cc: netdev, kuniyu
In-Reply-To: <20260328213338.450601-1-nickgarlis@gmail.com>
On Sat, 28 Mar 2026 22:33:38 +0100 Nikolaos Gkarlis wrote:
> -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])
There's an early return hiding outside of the context here.
the patch is technically correct, I think, because if we take this
shortcut we end up with the same netns as tgt_net so we'll validate
that it's capable later. But it's probably not obvious to a casual
reader of this code (or AI agents, sigh)
So let's rewrite this along the lines of:
struct nlattr *tb[IFLA_MAX + 1], **attrs;
struct net *net;
int err;
if (!data || !data[ops->peer_type]) {
attrs = tbp;
} else {
err = rtnl_nla_parse_ifinfomsg(tb, data[ops->peer_type], extack);
if (err < 0)
return ERR_PTR(err);
if (ops->validate) {
err = ops->validate(tb, NULL, extack);
if (err < 0)
return ERR_PTR(err);
}
attrs = tb;
}
net = rtnl_link_get_net_ifla(attrs);
if (IS_ERR_OR_NULL(net))
return net;
if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
...
?
> @@ -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;
^ permalink raw reply
* Re: [PATCH net v2] net/x25: Fix overflow when accumulating packets
From: Jakub Kicinski @ 2026-04-02 2:48 UTC (permalink / raw)
To: ms
Cc: Jakub Kicinski, davem, edumazet, pabeni, yimingqian591, linux-x25,
netdev, linux-kernel, security
In-Reply-To: <20260327-x25_fraglen-v2-1-143911c3f62a@dev.tdt.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.
---
net/x25: Fix overflow when accumulating packets
This commit adds a check to prevent x25_sock.fraglen from overflowing
during packet accumulation. It also resets the fragment length when
purging the fragment queue.
> diff --git a/net/x25/x25_in.c b/net/x25/x25_in.c
> index b981a4828d08..cb84c683d249 100644
> --- a/net/x25/x25_in.c
> +++ b/net/x25/x25_in.c
> @@ -34,6 +34,12 @@ static int x25_queue_rx_frame(struct sock *sk, struct sk_buff *skb, int more)
> struct sk_buff *skbo, *skbn = skb;
> struct x25_sock *x25 = x25_sk(sk);
>
> + /* make sure we don't overflow */
> + if (x25->fraglen + skb->len > USHRT_MAX) {
> + kfree_skb(skb);
> + return 1;
> + }
Does this cause the skb to be freed twice?
When x25_queue_rx_frame() frees the skb and returns 1, the caller
x25_state3_machine() leaves its local queued variable as 0 and returns 0.
This 0 value propagates up through x25_process_rx_frame() to callers
like x25_backlog_rcv() or x25_lapb_receive_frame(). Since the return
value indicates the packet was not consumed by the protocol stack, these
upper-level functions will call kfree_skb(skb) again on the already
freed skb.
Would it be safer to return 1 without calling kfree_skb(skb) here so
the callers can handle freeing the unconsumed packet?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH] NFC: s3fwrn5: bound the UART receive buffer
From: Jakub Kicinski @ 2026-04-02 2:49 UTC (permalink / raw)
To: pengpeng
Cc: Jakub Kicinski, netdev, krzk, bongsu.jeon, davem, edumazet,
pabeni, linux-kernel
In-Reply-To: <20260328113615.37260-1-pengpeng@iscas.ac.cn>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
NFC: s3fwrn5: bound the UART receive buffer
This commit attempts to fix a potential buffer overrun in the UART receive
handler by dropping oversized frames before they exceed NCI_SKB_BUFF_LEN.
> diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
> index ccfcee06ee77..aba688499c05 100644
> --- a/drivers/nfc/s3fwrn5/uart.c
> +++ b/drivers/nfc/s3fwrn5/uart.c
> @@ -63,8 +63,17 @@ static size_t s3fwrn82_uart_read(struct serdev_device *serdev,
[ ... ]
> if ((phy->recv_skb->len - S3FWRN82_NCI_HEADER)
> - < phy->recv_skb->data[S3FWRN82_NCI_IDX])
> + < phy->recv_skb->data[S3FWRN82_NCI_IDX]) {
> + if (phy->recv_skb->len < NCI_SKB_BUFF_LEN)
> + continue;
Is it mathematically possible for this condition to be false?
Since phy->recv_skb->data[S3FWRN82_NCI_IDX] is an 8-bit unsigned integer,
its maximum value is 255. With S3FWRN82_NCI_HEADER being 3, the check
len - 3 < data[2] ensures the length is strictly less than 258 when
entering this block.
Because NCI_SKB_BUFF_LEN is 258, wouldn't len < NCI_SKB_BUFF_LEN always
evaluate to true, making the new frame-dropping logic unreachable?
> +
> + dev_dbg(&serdev->dev, "dropping oversized UART frame\n");
> + kfree_skb(phy->recv_skb);
> + phy->recv_skb = alloc_skb(NCI_SKB_BUFF_LEN, GFP_KERNEL);
> + if (!phy->recv_skb)
> + return 0;
This isn't a new issue introduced by this patch, but does returning 0 here
after a failed allocation cause a NULL pointer dereference?
If alloc_skb() fails, phy->recv_skb becomes NULL and the function returns 0.
When the serdev core retries delivering the buffer later, the loop
immediately calls skb_put_u8(phy->recv_skb, *data++), which will dereference
the NULL pointer.
Also, does returning 0 after already processing some bytes corrupt the
UART state machine by forcing already-consumed bytes to be replayed?
> continue;
> + }
>
> s3fwrn5_recv_frame(phy->common.ndev, phy->recv_skb,
> phy->common.mode);
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net-next v7] selftests: net: add tests for PPP
From: Jakub Kicinski @ 2026-04-02 2:52 UTC (permalink / raw)
To: Qingfang Deng
Cc: Dianne Skoll, Shuah Khan, David S. Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, Felix Maurer,
Sebastian Andrzej Siewior, Matthieu Baerts (NGI0), linux-kernel,
linux-kselftest, linux-ppp, netdev, Paul Mackerras
In-Reply-To: <CALW65jaTbaosbO8ypKZ6G37HxuxXabeTaP_Fhq72xTovmDj+8g@mail.gmail.com>
On Thu, 2 Apr 2026 10:26:11 +0800 Qingfang Deng wrote:
> Hi,
>
> On Thu, Apr 2, 2026 at 8:56 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Wed, 1 Apr 2026 23:45:38 +0800 Qingfang Deng wrote:
> [...]
> > >
> > > It looks like pppoe-server fails to start. You may check the syslog to
> > > see what's going on.
> >
> > Hm, we don't capture syslog automatically :S We capture stdout and
> > stderr
> >
> > I'm not even sure there's something listening on syslog in virtme-ng
> > Is it possible to make the daemon log to stdout or a file and dump
> > that file to stdout on failure? Even if we fix the current issue
> > it will be quite inconvenient if we have to go into syslog every time
> > the test is failing. The test output should provide enough info to
> > debug failures.
>
> Add Cc: Dianne
>
> I checked the man page of pppoe-server and did not find such option.
> We can instead start our own listener and redirect the syslog to a
> file or stdout:
>
> socat -u UNIX-RECV:/dev/log,ignoreeof STDOUT &
That should work for vng, I confirmed it doesn't seem to have a syslog
socket
$ vng -r
...
$ ls /dev/log
ls: cannot access '/dev/log': No such file or directory
^ permalink raw reply
* Re: [PATCH v2] selftests/bpf: Reject malformed IPv4/IPv6 skb test input
From: sun jian @ 2026-04-02 2:54 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: ast, daniel, andrii, eddyz87, song, yonghong.song, john.fastabend,
kpsingh, sdf, haoluo, jolsa, davem, edumazet, kuba, pabeni, horms,
bpf, netdev, linux-kernel, syzbot+619b9ef527f510a57cfc
In-Reply-To: <ac2ccb5NBAB3Zq1D@devbig1721.ftw5.facebook.com>
On Thu, Apr 2, 2026 at 10:17 AM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On Mon, Mar 30, 2026 at 12:17:51AM +0800, Sun Jian wrote:
> > bpf_prog_test_run_skb() derives skb->protocol from the Ethernet header
> > through eth_type_trans(), but it does not verify that the provided
> > linear input is long enough to contain the corresponding L3 base header.
> >
> > This can result in an inconsistent skb being passed to test_run helpers
> > such as bpf_skb_adjust_room(), where inferred protocol offsets can lead
> > to operating on uninitialized memory, triggering KMSAN errors.
> >
> > To reject such malformed test input, we check that the linear head is
> > sufficiently large to contain the corresponding L3 base header (IPv4
> > or IPv6) before running the program.
> >
> > Reported-by: syzbot+619b9ef527f510a57cfc@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=619b9ef527f510a57cfc
> > Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> > ---
> > v2:
> > - Ensured that the linear head is large enough to accommodate the corresponding L3 base header (IPv4 or IPv6), before running the program.
> >
> > Link: <https://lore.kernel.org/bpf/129d235b04aca276c0a57c7c3646ce48644458cdc85d9b92b25f405e2d58a9ae@mail.kernel.org/>
> >
> > net/bpf/test_run.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > index 178c4738e63b..4790bee535b9 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -1118,6 +1118,25 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> > skb->protocol = eth_type_trans(skb, dev);
> > skb_reset_network_header(skb);
> >
> > + switch (skb->protocol) {
>
> The implementation is poor.
>
> > + case htons(ETH_P_IP):
> > + if (skb_headlen(skb) < sizeof(struct iphdr)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + break;
> > +#if IS_ENABLED(CONFIG_IPV6)
> > + case htons(ETH_P_IPV6):
> > + if (skb_headlen(skb) < sizeof(struct ipv6hdr)) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + break;
> > +#endif
> > + default:
> > + break;
> > + }
> > +
> > switch (skb->protocol) {
>
> There is an exact same switch and it has the same check
> on skb_headlen(skb).
>
> A selftest is needed. Check if the tests in empty_skb.c need to be
> changed also. imo, This can be bpf-next.
>
> pw-bot: cr
>
> > case htons(ETH_P_IP):
> > sk->sk_family = AF_INET;
> >
> > base-commit: cbfffcca2bf0622b601b7eaf477aa29035169184
> > --
> > 2.43.0
> >
Ack, I'll respin a v2.
BTW, v1 was mainly meant as a minimal proof of the fix, so I
kept the existing structure intact.
Sun Jian
^ permalink raw reply
* Re: [PATCH net v6 0/4] macsec: Add support for VLAN filtering in offload mode
From: Jakub Kicinski @ 2026-04-02 2:54 UTC (permalink / raw)
To: Sabrina Dubroca
Cc: Cosmin Ratiu, netdev, Andrew Lunn, David S . Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, Stanislav Fomichev, Shuah Khan,
linux-kselftest, Dragos Tatulea
In-Reply-To: <20260330130130.989236-1-cratiu@nvidia.com>
On Mon, 30 Mar 2026 16:01:26 +0300 Cosmin Ratiu wrote:
> This short series adds support for VLANs in MACsec devices when offload
> mode is enabled. This allows VLAN netdevs on top of MACsec netdevs to
> function, which accidentally used to be the case in the past, but was
> broken. This series adds back proper support.
>
> As part of this, the existing nsim-only MACsec offload tests were
> translated to Python so they can run against real HW and new
> traffic-based tests were added for VLAN filter propagation, since
> there's currently no uAPI to check VLAN filters.
Hi Sabrina, how does this look to you?
^ permalink raw reply
* Re: [PATCH RESEND net v2 1/1] net/sched: sch_netem: fix out-of-bounds access in packet corruption
From: patchwork-bot+netdevbpf @ 2026-04-02 3:00 UTC (permalink / raw)
To: Yucheng Lu
Cc: security, stephen, netdev, jhs, jiri, davem, Jason, kees,
yifanwucs, tomapufckgml, tanyuan98, bird, z1652074432
In-Reply-To: <45435c0935df877853a81e6d06205ac738ec65fa.1774941614.git.kanolyc@gmail.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 31 Mar 2026 16:00:21 +0800 you wrote:
> In netem_enqueue(), the packet corruption logic uses
> get_random_u32_below(skb_headlen(skb)) to select an index for
> modifying skb->data. When an AF_PACKET TX_RING sends fully non-linear
> packets over an IPIP tunnel, skb_headlen(skb) evaluates to 0.
>
> Passing 0 to get_random_u32_below() takes the variable-ceil slow path
> which returns an unconstrained 32-bit random integer. Using this
> unconstrained value as an offset into skb->data results in an
> out-of-bounds memory access.
>
> [...]
Here is the summary with links:
- [RESEND,net,v2,1/1] net/sched: sch_netem: fix out-of-bounds access in packet corruption
https://git.kernel.org/netdev/net/c/d64cb81dcbd5
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 net,v5] virtio_net: clamp rss_max_key_size to NETDEV_RSS_KEY_LEN
From: patchwork-bot+netdevbpf @ 2026-04-02 3:00 UTC (permalink / raw)
To: Srujana Challa
Cc: netdev, virtualization, pabeni, mst, jasowang, xuanzhuo, eperezma,
davem, edumazet, kuba, ndabilpuram, kshankar, stable
In-Reply-To: <20260326142344.1171317-1-schalla@marvell.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 26 Mar 2026 19:53:44 +0530 you wrote:
> rss_max_key_size in the virtio spec is the maximum key size supported by
> the device, not a mandatory size the driver must use. Also the value 40
> is a spec minimum, not a spec maximum.
>
> The current code rejects RSS and can fail probe when the device reports a
> larger rss_max_key_size than the driver buffer limit. Instead, clamp the
> effective key length to min(device rss_max_key_size, NETDEV_RSS_KEY_LEN)
> and keep RSS enabled.
>
> [...]
Here is the summary with links:
- [net,v5] virtio_net: clamp rss_max_key_size to NETDEV_RSS_KEY_LEN
https://git.kernel.org/netdev/net/c/b4e5f04c58a2
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 net 1/2] net: macb: fix clk handling on PCI glue driver removal
From: patchwork-bot+netdevbpf @ 2026-04-02 3:00 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: nicolas.ferre, claudiu.beznea, theo.lebrun, andrew+netdev, davem,
edumazet, kuba, pabeni, ztong0001, bfolta, netdev, linux-kernel,
lvc-project
In-Reply-To: <20260330184542.626619-1-pchelkin@ispras.ru>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 30 Mar 2026 21:45:40 +0300 you wrote:
> platform_device_unregister() may still want to use the registered clks
> during runtime resume callback.
>
> Note that there is a commit d82d5303c4c5 ("net: macb: fix use after free
> on rmmod") that addressed the similar problem of clk vs platform device
> unregistration but just moved the bug to another place.
>
> [...]
Here is the summary with links:
- [net,1/2] net: macb: fix clk handling on PCI glue driver removal
https://git.kernel.org/netdev/net/c/ce8fe5287b87
- [net,2/2] net: macb: properly unregister fixed rate clocks
https://git.kernel.org/netdev/net/c/f0f367a4f459
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 net 1/3] net/mlx5e: SD, Fix race condition in secondary device probe/remove
From: Jakub Kicinski @ 2026-04-02 3:08 UTC (permalink / raw)
To: Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Saeed Mahameed, Mark Bloch, Leon Romanovsky, Shay Drory,
Simon Horman, Kees Cook, Parav Pandit, Patrisious Haddad,
Gal Pressman, netdev, linux-rdma, linux-kernel
In-Reply-To: <20260330193412.53408-2-tariqt@nvidia.com>
On Mon, 30 Mar 2026 22:34:10 +0300 Tariq Toukan wrote:
> From: Shay Drory <shayd@nvidia.com>
>
> When utilizing Socket-Direct single netdev functionality the driver
> resolves the actual auxiliary device using mlx5_sd_get_adev(). However,
> the current implementation returns the primary ETH auxiliary device
> without holding the device lock, leading to a potential race condition
> where the ETH device could be unbound or removed concurrently during
> probe, suspend, resume, or remove operations.[1]
>
> Fix this by introducing mlx5_sd_put_adev() and updating
> mlx5_sd_get_adev() so that secondaries devices would acquire the device
> lock of the returned auxiliary device. After the lock is acquired, a
> second devcom check is needed[2].
> In addition, update The callers to pair the get operation with the new
> put operation, ensuring the lock is held while the auxiliary device is
> being operated on and released afterwards.
Please explain why the "primary" designation is reliable, and therefore
we can be sure there will be no ABBA deadlock here
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index b6c12460b54a..5761f655f488 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -6657,8 +6657,11 @@ static int mlx5e_resume(struct auxiliary_device *adev)
> return err;
>
> actual_adev = mlx5_sd_get_adev(mdev, adev, edev->idx);
> - if (actual_adev)
> - return _mlx5e_resume(actual_adev);
> + if (actual_adev) {
> + err = _mlx5e_resume(actual_adev);
> + mlx5_sd_put_adev(actual_adev, adev);
> + return err;
> + }
> return 0;
Feels like I recently complained about similar code y'all were trying
to add. Magically and conditionally locking something in a get helper
makes for extremely confusing code.
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net 2/3] net/mlx5: SD, Keep multi-pf debugfs entries on primary
From: Jakub Kicinski @ 2026-04-02 3:09 UTC (permalink / raw)
To: tariqt
Cc: Jakub Kicinski, edumazet, pabeni, andrew+netdev, davem, saeedm,
mbloch, leon, shayd, horms, kees, parav, phaddad, gal, netdev,
linux-rdma, linux-kernel
In-Reply-To: <20260330193412.53408-3-tariqt@nvidia.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/mlx5: SD, Keep multi-pf debugfs entries on primary
This commit fixes an issue where the "multi-pf" debugfs directory was stored
in the calling device's sd struct but created under the primary device's root.
It explicitly stores the debugfs dentry in the primary device's sd struct.
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> index 060649645012..4c80b9d25283 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
[ ... ]
> @@ -491,6 +499,7 @@ void mlx5_sd_cleanup(struct mlx5_core_dev *dev)
> {
> struct mlx5_sd *sd = mlx5_get_sd(dev);
> struct mlx5_core_dev *primary, *pos;
> + struct mlx5_sd *primary_sd;
> int i;
>
> if (!sd)
> @@ -500,10 +509,12 @@ void mlx5_sd_cleanup(struct mlx5_core_dev *dev)
> goto out;
>
> primary = mlx5_sd_get_primary(dev);
> + primary_sd = mlx5_get_sd(primary);
> mlx5_sd_for_each_secondary(i, primary, pos)
> sd_cmd_unset_secondary(pos);
> sd_cmd_unset_primary(primary);
> - debugfs_remove_recursive(sd->dfs);
> + debugfs_remove_recursive(primary_sd->dfs);
> + primary_sd->dfs = NULL;
Can concurrent unbind operations cause a use-after-free and double-free
regression here?
In mlx5_sd_cleanup(), the check !mlx5_devcom_comp_is_ready(sd->devcom) is
performed locklessly. If multiple physical functions in the Socket-Direct
group are unbound concurrently, multiple threads can observe ready == true
and enter this teardown block simultaneously.
Since the cleanup code now operates on the shared primary_sd pointer instead
of the local device's sd, the thread for the primary physical function might
complete its unbind and execute sd_cleanup(dev), freeing its sd object
(which is primary_sd).
Concurrently, the thread for a secondary physical function will dereference
the freed primary_sd to access primary_sd->dfs, causing a use-after-free.
Both threads could also pass the same pointer to debugfs_remove_recursive(),
resulting in a double-free.
Note: This was fixed later in the patch series by commit 92528cec4e0b91deb
("net/mlx5: SD: Serialize init/cleanup").
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net 3/3] net/mlx5: SD: Serialize init/cleanup
From: Jakub Kicinski @ 2026-04-02 3:09 UTC (permalink / raw)
To: tariqt
Cc: Jakub Kicinski, edumazet, pabeni, andrew+netdev, davem, saeedm,
mbloch, leon, shayd, horms, kees, parav, phaddad, gal, netdev,
linux-rdma, linux-kernel
In-Reply-To: <20260330193412.53408-4-tariqt@nvidia.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/mlx5: SD: Serialize init/cleanup
This commit introduces a state tracker and locking around the Socket-Direct
init and cleanup routines. It prevents multiple physical functions from
concurrently bringing up or tearing down the SD group.
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> index 4c80b9d25283..374f27b78fbe 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
[ ... ]
> @@ -505,11 +520,16 @@ void mlx5_sd_cleanup(struct mlx5_core_dev *dev)
> if (!sd)
> return;
>
> + mlx5_devcom_comp_lock(sd->devcom);
> if (!mlx5_devcom_comp_is_ready(sd->devcom))
> - goto out;
> + goto out_unlock;
>
> primary = mlx5_sd_get_primary(dev);
> primary_sd = mlx5_get_sd(primary);
> +
> + if (primary_sd->state != MLX5_SD_STATE_UP)
> + goto out_unlock;
> +
> mlx5_sd_for_each_secondary(i, primary, pos)
> sd_cmd_unset_secondary(pos);
> sd_cmd_unset_primary(primary);
> @@ -517,7 +537,9 @@ void mlx5_sd_cleanup(struct mlx5_core_dev *dev)
> primary_sd->dfs = NULL;
>
> sd_info(primary, "group id %#x, uncombined\n", sd->group_id);
> -out:
> + primary_sd->state = MLX5_SD_STATE_DOWN;
> +out_unlock:
> + mlx5_devcom_comp_unlock(sd->devcom);
> sd_unregister(dev);
> sd_cleanup(dev);
> }
Is there a race condition if a secondary PF calls mlx5_sd_init() while the
primary PF is running mlx5_sd_cleanup()?
In mlx5_sd_cleanup(), the code sets the state to MLX5_SD_STATE_DOWN and drops
the devcom lock before calling sd_unregister(). At this point, the primary
PF is still registered in the devcom group, so mlx5_devcom_comp_is_ready()
remains true.
If a secondary PF is concurrently probed, could it acquire the devcom lock
and observe this intermediate state?
CPU1
mlx5_sd_cleanup()
primary_sd->state = MLX5_SD_STATE_DOWN;
mlx5_devcom_comp_unlock(sd->devcom);
// Preempted before sd_unregister(dev)
CPU2 would see this in mlx5_sd_init():
mlx5_devcom_comp_lock(sd->devcom);
if (!mlx5_devcom_comp_is_ready(sd->devcom)) // returns true
goto out;
primary = mlx5_sd_get_primary(dev);
primary_sd = mlx5_get_sd(primary);
if (primary_sd->state == MLX5_SD_STATE_UP) // returns false
goto out;
...
debugfs_create_file("primary", 0400, primary_sd->dfs, primary, &dev_fops);
Would CPU2 proceed to initialize the group using the dying primary PF?
If CPU1 then resumes and finishes sd_cleanup(), it would free the primary_sd
structure and the primary device. This might leave the secondary PF with
dangling pointers in the debugfs files, potentially leading to a
use-after-free on access.
Furthermore, when the secondary is later removed, mlx5_sd_cleanup(secondary)
might see is_ready as false and skip the teardown entirely, leaving the
dangling debugfs files permanently.
^ permalink raw reply
* Re: [PATCH net-next v11 0/6] tls: Add TLS 1.3 hardware offload support
From: Jakub Kicinski @ 2026-04-02 3:16 UTC (permalink / raw)
To: Rishikesh Jethwani
Cc: netdev, saeedm, tariqt, mbloch, borisp, john.fastabend, sd, davem,
pabeni, edumazet, leon
In-Reply-To: <20260331155859.1f34ef38@kernel.org>
On Tue, 31 Mar 2026 15:58:59 -0700 Jakub Kicinski wrote:
> On Tue, 31 Mar 2026 10:37:51 -0600 Rishikesh Jethwani wrote:
> > Changes in v11:
> > - tls_device_complete_rekey(): flush pending open_rec (from MSG_MORE)
> > via tls_sw_push_pending_record() before switching back to HW offload.
> > Without this, data in an open SW record would be silently lost when
> > the AEAD cipher is freed.
> > - Selftest: enforce MIN_BUF_SIZE (16 bytes) on both client and server
> > receive buffers to prevent KeyUpdate handshake message truncation
> > with small -b values.
>
> Not more than 1 version in a 24h period, please. No matter how trivial
> the reason.
Series does not apply.
--
pw-bot: cr
^ permalink raw reply
* RE: [PATCH net v2] tipc: fix bc_ackers underflow on duplicate GRP_ACK_MSG
From: Tung Quang Nguyen @ 2026-04-02 3:16 UTC (permalink / raw)
To: Oleh Konko
Cc: jmaloy@redhat.com, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
tipc-discussion@lists.sourceforge.net,
linux-kernel@vger.kernel.org, stable@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <d72c0ff783db4c78ad862e6e27f3a807.security@1seal.org>
>Subject: [PATCH net v2] tipc: fix bc_ackers underflow on duplicate
>GRP_ACK_MSG
>
>The GRP_ACK_MSG handler in tipc_group_proto_rcv() currently decrements
>bc_ackers on every inbound group ACK, even when the same member has
>already acknowledged the current broadcast round.
>
>Because bc_ackers is a u16, a duplicate ACK received after the last legitimate
>ACK wraps the counter to 65535. Once wrapped,
>tipc_group_bc_cong() keeps reporting congestion and later group broadcasts
>on the affected socket stay blocked until the group is recreated.
>
>Fix this by ignoring duplicate or stale ACKs before touching bc_acked or
>bc_ackers. This makes repeated GRP_ACK_MSG handling idempotent and
>prevents the underflow path.
>
>Fixes: 75da2163dbb6 ("tipc: introduce communication groups")
AI complains that above Fixes tag is not correct:
" The Fixes: tag appears to reference the wrong commit. The bug was actually
introduced by commit 2f487712b893 ("tipc: guarantee that group broadcast
doesn't bypass group unicast").
Commit 75da2163dbb6 introduced the basic group communication feature but did
not include bc_ackers or GRP_ACK_MSG handling. Commit 2f487712b893 added the
GRP_ACK_MSG handler with the buggy unconditional decrement:
if (--grp->bc_ackers)
Should the Fixes: tag be updated to:
Fixes: 2f487712b893 ("tipc: guarantee that group broadcast doesn't bypass group unicast")
"
I agree with AI review result. Could you please correct the Fixes tag ?
>Cc: stable@vger.kernel.org
>Signed-off-by: Oleh Konko <security@1seal.org>
>---
>v2:
>- make duplicate or stale GRP_ACK_MSG a full no-op via early return
>- place acked in reverse xmas tree style
>
> net/tipc/group.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/net/tipc/group.c b/net/tipc/group.c index
>e0e6227b433..14e6732624e 100644
>--- a/net/tipc/group.c
>+++ b/net/tipc/group.c
>@@ -746,6 +746,7 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool
>*usr_wakeup,
> u32 port = msg_origport(hdr);
> struct tipc_member *m, *pm;
> u16 remitted, in_flight;
>+ u16 acked;
>
> if (!grp)
> return;
>@@ -798,7 +799,10 @@ void tipc_group_proto_rcv(struct tipc_group *grp,
>bool *usr_wakeup,
> case GRP_ACK_MSG:
> if (!m)
> return;
>- m->bc_acked = msg_grp_bc_acked(hdr);
>+ acked = msg_grp_bc_acked(hdr);
>+ if (less_eq(acked, m->bc_acked))
>+ return;
>+ m->bc_acked = acked;
> if (--grp->bc_ackers)
> return;
> list_del_init(&m->small_win);
>--
>2.50.0
^ permalink raw reply
* Re: [PATCH net 0/3] mlx5 misc fixes 2026-03-30
From: patchwork-bot+netdevbpf @ 2026-04-02 3:20 UTC (permalink / raw)
To: Tariq Toukan
Cc: edumazet, kuba, pabeni, andrew+netdev, davem, saeedm, leon,
mbloch, shayd, shayag, saeedm, jianbol, netdev, linux-rdma,
linux-kernel, gal
In-Reply-To: <20260330194015.53585-1-tariqt@nvidia.com>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 30 Mar 2026 22:40:12 +0300 you wrote:
> Hi,
>
> This patchset provides misc bug fixes from the team to the mlx5
> core driver.
>
> Thanks,
> Tariq.
>
> [...]
Here is the summary with links:
- [net,1/3] net/mlx5: lag: Check for LAG device before creating debugfs
https://git.kernel.org/netdev/net/c/bf16bca66536
- [net,2/3] net/mlx5: Avoid "No data available" when FW version queries fail
https://git.kernel.org/netdev/net/c/10dc35f6a443
- [net,3/3] net/mlx5: Fix switchdev mode rollback in case of failure
https://git.kernel.org/netdev/net/c/403186400a1a
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 net v2 0/3] bnxt_en: Bug fixes
From: patchwork-bot+netdevbpf @ 2026-04-02 3:20 UTC (permalink / raw)
To: Michael Chan
Cc: davem, netdev, edumazet, kuba, pabeni, andrew+netdev,
pavan.chebbi, andrew.gospodarek
In-Reply-To: <20260331065138.948205-1-michael.chan@broadcom.com>
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 30 Mar 2026 23:51:35 -0700 you wrote:
> The first patch is a refactor patch needed by the second patch to
> fix XDP ring initialization during FW reset. The third patch
> fixes an issue related to stats context reservation for RoCE.
>
> v2: Fix Fixes tag commit ID for patch #2
>
> v1: https://lore.kernel.org/netdev/20260329232528.870637-1-michael.chan@broadcom.com/
>
> [...]
Here is the summary with links:
- [net,v2,1/3] bnxt_en: Refactor some basic ring setup and adjustment logic
https://git.kernel.org/netdev/net/c/ceee35e5674a
- [net,v2,2/3] bnxt_en: Don't assume XDP is never enabled in bnxt_init_dflt_ring_mode()
https://git.kernel.org/netdev/net/c/e4bf81dcad0a
- [net,v2,3/3] bnxt_en: Restore default stat ctxs for ULP when resource is available
https://git.kernel.org/netdev/net/c/071dbfa304e8
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 net-next] Octeontx2-af: add WQ_PERCPU to alloc_workqueue users
From: patchwork-bot+netdevbpf @ 2026-04-02 3:20 UTC (permalink / raw)
To: Marco Crivellari
Cc: linux-kernel, netdev, tj, jiangshanlai, frederic, bigeasy, mhocko,
sgoutham, lcherian, gakula, jerinj, hkelam, sbhatta,
andrew+netdev, davem, edumazet, kuba, pabeni
In-Reply-To: <20260330095405.116990-1-marco.crivellari@suse.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 30 Mar 2026 11:54:05 +0200 you wrote:
> This continues the effort to refactor workqueue APIs, which began with
> the introduction of new workqueues and a new alloc_workqueue flag in:
>
> commit 128ea9f6ccfb ("workqueue: Add system_percpu_wq and system_dfl_wq")
> commit 930c2ea566af ("workqueue: Add new WQ_PERCPU flag")
>
> The refactoring is going to alter the default behavior of
> alloc_workqueue() to be unbound by default.
>
> [...]
Here is the summary with links:
- [net-next] Octeontx2-af: add WQ_PERCPU to alloc_workqueue users
https://git.kernel.org/netdev/net-next/c/a77fb1ace44e
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] net: microchip: dead code cleanup in kconfig for FDMA
From: patchwork-bot+netdevbpf @ 2026-04-02 3:20 UTC (permalink / raw)
To: Julian Braha
Cc: pabeni, davem, daniel.machon, andrew+netdev, edumazet, kuba,
linux-kernel, netdev
In-Reply-To: <20260329185348.526893-1-julianbraha@gmail.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Sun, 29 Mar 2026 19:53:48 +0100 you wrote:
> 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
>
> [...]
Here is the summary with links:
- net: microchip: dead code cleanup in kconfig for FDMA
https://git.kernel.org/netdev/net-next/c/48c6255cda81
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 net-next] selftests/net: Add two xdp tests to xdp.py
From: Leon Hwang @ 2026-04-02 3:22 UTC (permalink / raw)
To: Jakub Kicinski, Leon Hwang
Cc: netdev, Andrew Lunn, David S . Miller, Eric Dumazet, Paolo Abeni,
Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
linux-kselftest, linux-kernel, bpf
In-Reply-To: <20260401132048.3a15601e@kernel.org>
On 2/4/26 04:20, Jakub Kicinski wrote:
> On Wed, 1 Apr 2026 13:27:46 +0800 Leon Hwang wrote:
[...]
>> +def _set_jumbo_mtu(cfg, mtu):
>> + ip(f"link set dev {cfg.ifname} mtu {mtu}")
>> + defer(ip, f"link set dev {cfg.ifname} mtu 1500")
>
> cfg records the original MTU, you can copy this line from here:
> https://elixir.bootlin.com/linux/v7.0-rc6/source/tools/testing/selftests/drivers/net/gro.py#L88
>
Sure, will use cfg.dev['mtu'] instead of hard code 1500.
>> +def _exec_cmd(cfg, obj, sec, ip_opts=""):
>> + return cmd(f"ip {ip_opts} link set dev {cfg.ifname} xdpdrv obj {obj} sec {sec}", shell=True, fail=False)
>
> We shouldn't need the shell=True where?
> Also it's probably cleaner to pass fail as optional argument here
> so that you can let this throw an exception in the cases we expect
> to succeed instead of having to check in the caller explicitly
>
Right, shell=True is unnecessary here. Will drop it.
Will pass fail as optional arg to let it fail if fail=True.
>> +def test_xdp_native_attach_sb_to_mb(cfg):
>> + obj = cfg.net_lib_dir / "xdp_dummy.bpf.o"
>> + mtu = 9000
>> +
>> + _set_jumbo_mtu(cfg, mtu)
>> +
>> + probe = _exec_cmd(cfg, obj, "xdp.frags")
>> + if probe.ret != 0:
>> + output = probe.stderr.strip() or probe.stdout.strip()
>> + raise KsftSkipEx(output or "device does not support multi-buffer XDP")
>
> May be simpler and cleaner to print the whole command separately:
>
> if probe.ret != 0:
> ksft_pr(probe)
> raise KsftSkipEx("device does not support multi-buffer XDP")
>
Ack.
>> + ip(f"link set dev {cfg.ifname} xdpdrv off")
>> +
>> + probe = _exec_cmd(cfg, obj, "xdp")
>> + if probe.ret == 0:
>> + ip(f"link set dev {cfg.ifname} xdpdrv off")
>> + raise KsftFailEx(f"driver unexpectedly allows non-multi-buffer XDP at MTU {mtu}")
>
> Hm, TBH I don't think this sb_to_mb case is adding any coverage.
> Let's just add the test case below?
>
Ok. Will drop this case.
>
>> +def test_xdp_native_update_mb_to_sb(cfg):
>> + obj = cfg.net_lib_dir / "xdp_dummy.bpf.o"
>> +
>> + _set_jumbo_mtu(cfg, 9000)
>> +
>> + attach = _exec_cmd(cfg, obj, "xdp.frags")
>> + if attach.ret != 0:
>> + output = attach.stderr.strip() or attach.stdout.strip()
>> + raise KsftSkipEx(output or "device does not support multi-buffer XDP")
>> +
>> + defer(ip, f"link set dev {cfg.ifname} xdpdrv off")
>> +
>> + update1 = _exec_cmd(cfg, obj, "xdp.frags", "-force")
>> + if update1.ret != 0:
>> + raise KsftFailEx("device fails to update multi-buffer XDP")
>> +
>> + update2 = _exec_cmd(cfg, obj, "xdp", "-force")
>> + if update2.ret == 0:
>> + raise KsftFailEx("device unexpectedly updates non-multi-buffer XDP")
From sashiko's review [1]:
On architectures with larger page sizes, such as 16KB or 64KB on ARM64 or
PowerPC, a 9000-byte packet can fit entirely within a single buffer. In
these environments, the driver might legally accept a non-frag XDP program,
which would lead to a false positive failure here.
WDYT?
[1]
https://sashiko.dev/#/patchset/20260401052746.314667-1-leon.huangfu%40shopee.com
Thanks,
Leon
^ permalink raw reply
* Re: [PATCH net-next v5 3/6] net: bcmgenet: add basic XDP support (PASS/DROP)
From: Jakub Kicinski @ 2026-04-02 3:27 UTC (permalink / raw)
To: Nicolai Buchwitz
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: <20260328230513.415790-4-nb@tipi-net.de>
On Sun, 29 Mar 2026 00:05:06 +0100 Nicolai Buchwitz wrote:
> @@ -2403,26 +2456,52 @@ static unsigned int bcmgenet_desc_rx(struct bcmgenet_rx_ring *ring,
> goto next;
> } /* error packet */
>
> - /* Build SKB from the page - data starts at hard_start,
> - * frame begins after RSB(64) + pad(2) = 66 bytes.
> - */
> - skb = napi_build_skb(hard_start, PAGE_SIZE - GENET_XDP_HEADROOM);
> - if (unlikely(!skb)) {
> - BCMGENET_STATS64_INC(stats, dropped);
> - page_pool_put_full_page(ring->page_pool, rx_page,
> - true);
> - goto next;
> - }
> -
> - skb_mark_for_recycle(skb);
> + /* XDP: frame data starts after RSB + pad */
> + if (xdp_prog) {
> + struct xdp_buff xdp;
> + unsigned int xdp_act;
> + int pkt_len;
> +
> + pkt_len = len - GENET_RSB_PAD;
> + if (priv->crc_fwd_en)
> + pkt_len -= ETH_FCS_LEN;
> +
> + xdp_init_buff(&xdp, PAGE_SIZE, &ring->xdp_rxq);
> + xdp_prepare_buff(&xdp, page_address(rx_page),
> + GENET_RX_HEADROOM, pkt_len, true);
> +
> + xdp_act = bcmgenet_run_xdp(ring, xdp_prog, &xdp,
> + rx_page);
> + if (xdp_act != XDP_PASS)
> + goto next;
> +
> + /* XDP_PASS: build SKB from (possibly modified) xdp */
> + skb = bcmgenet_xdp_build_skb(ring, &xdp, rx_page);
> + if (unlikely(!skb)) {
> + BCMGENET_STATS64_INC(stats, dropped);
> + page_pool_put_full_page(ring->page_pool,
> + rx_page, true);
> + goto next;
> + }
> + } else {
> + /* Build SKB from the page - data starts at
> + * hard_start, frame begins after RSB(64) + pad(2).
> + */
> + skb = napi_build_skb(hard_start,
> + PAGE_SIZE - GENET_XDP_HEADROOM);
> + if (unlikely(!skb)) {
> + BCMGENET_STATS64_INC(stats, dropped);
> + page_pool_put_full_page(ring->page_pool,
> + rx_page, true);
> + goto next;
> + }
The large branches here are quite unusual, normally drivers fully
prepare the xdp_buff and if there's no xdp prog attached act as
if there was one and it returned XDP_PASS. Saves LoC and therefore bugs.
>
> - /* Reserve the RSB + pad, then set the data length */
> - skb_reserve(skb, GENET_RSB_PAD);
> - __skb_put(skb, len - GENET_RSB_PAD);
> + skb_mark_for_recycle(skb);
> + skb_reserve(skb, GENET_RSB_PAD);
> + __skb_put(skb, len - GENET_RSB_PAD);
>
> - if (priv->crc_fwd_en) {
> - skb_trim(skb, skb->len - ETH_FCS_LEN);
> - len -= ETH_FCS_LEN;
> + if (priv->crc_fwd_en)
> + skb_trim(skb, skb->len - ETH_FCS_LEN);
> }
>
> /* Set up checksum offload */
AI points out that :
The status_64 structure is located at the start of the page
before the frame data. Since this resides inside the XDP headroom, if an XDP
program expands the header backwards (e.g., via bpf_xdp_adjust_head() or
metadata via bpf_xdp_adjust_meta()), could it physically overwrite
status->rx_csum?
Since the driver reads status->rx_csum after executing the XDP program, it
could risk reading garbage data if the headroom was used. Should the driver
skip setting CHECKSUM_COMPLETE when an XDP program is loaded?
> @@ -3745,6 +3824,39 @@ static int bcmgenet_change_carrier(struct net_device *dev, bool new_carrier)
> return 0;
> }
>
> +static int bcmgenet_xdp_setup(struct net_device *dev,
> + struct netdev_bpf *xdp)
> +{
> + struct bcmgenet_priv *priv = netdev_priv(dev);
> + struct bpf_prog *old_prog;
> + struct bpf_prog *prog = xdp->prog;
> +
> + if (prog && dev->mtu > PAGE_SIZE - GENET_RX_HEADROOM -
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))) {
> + NL_SET_ERR_MSG_MOD(xdp->extack,
> + "MTU too large for single-page XDP buffer");
> + return -EOPNOTSUPP;
> + }
> +
> + old_prog = xchg(&priv->xdp_prog, prog);
> + if (old_prog) {
> + synchronize_net();
Why? BPF prog gets freed after a RCU grace period I think?
> + bpf_prog_put(old_prog);
^ permalink raw reply
* Re: [PATCH net-next v5 4/6] net: bcmgenet: add XDP_TX support
From: Jakub Kicinski @ 2026-04-02 3:27 UTC (permalink / raw)
To: nb
Cc: Jakub Kicinski, netdev, justin.chen, horms, opendmb,
florian.fainelli, bcm-kernel-feedback-list, andrew+netdev, davem,
edumazet, pabeni, ast, daniel, hawk, john.fastabend, sdf,
linux-kernel, bpf
In-Reply-To: <20260328230513.415790-5-nb@tipi-net.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.
---
net: bcmgenet: add XDP_TX support
This commit implements XDP_TX using ring 16, which is dedicated to XDP TX
for isolation from SKB TX queues. It also changes the page_pool DMA
direction to bidirectional to allow TX reuse of the existing DMA mapping.
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index b45ba2c2857e..687c3b12d44f 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
[ ... ]
> @@ -2299,11 +2328,96 @@ static struct sk_buff *bcmgenet_xdp_build_skb(struct bcmgenet_rx_ring *ring,
> return skb;
> }
>
> +static bool bcmgenet_xdp_xmit_frame(struct bcmgenet_priv *priv,
> + struct xdp_frame *xdpf, bool dma_map)
> +{
[ ... ]
> + } else {
> + struct page *page = virt_to_page(xdpf->data);
> +
> + /* For local XDP_TX the caller already prepended the TSB
> + * into xdpf->data/len, so dma_len == xdpf->len.
> + */
> + dma_len = xdpf->len;
> + mapping = page_pool_get_dma_addr(page) +
> + sizeof(*xdpf) + xdpf->headroom;
When the RX page pool is created in bcmgenet_rx_ring_create_pool(), it is
initialized with .offset = GENET_XDP_HEADROOM. Does the DMA address
calculation here need to include this GENET_XDP_HEADROOM offset? Without it,
it appears the hardware might be given a DMA address that is 256 bytes
before the actual packet data, transmitting uninitialized memory.
[ ... ]
> @@ -2311,14 +2425,42 @@ static unsigned int bcmgenet_run_xdp(struct bcmgenet_rx_ring *ring,
> switch (act) {
> case XDP_PASS:
> return XDP_PASS;
> + case XDP_TX:
> + /* Prepend a zeroed TSB (Transmit Status Block). The GENET
> + * MAC has TBUF_64B_EN set globally, so hardware expects every
> + * TX buffer to begin with a 64-byte struct status_64. Back
> + * up xdp->data into the RSB area (which is no longer needed
> + * after the BPF program ran) and zero it.
> + */
> + if (xdp->data - xdp->data_hard_start <
> + sizeof(struct status_64)) {
> + page_pool_put_full_page(ring->page_pool, rx_page,
> + true);
> + return XDP_DROP;
> + }
> + xdp->data -= sizeof(struct status_64);
> + xdp->data_meta -= sizeof(struct status_64);
> + memset(xdp->data, 0, sizeof(struct status_64));
> +
> + xdpf = xdp_convert_buff_to_frame(xdp);
Does this headroom check account for the additional sizeof(struct xdp_frame)
required by xdp_convert_buff_to_frame()?
If metadata is present, xdp->data_meta could potentially underflow
xdp->data_hard_start. If the remaining headroom minus metasize becomes
negative in xdp_convert_buff_to_frame(), could it undergo integer promotion
to a large unsigned value and bypass the internal safety check? This might
cause the struct xdp_frame to overwrite the newly zeroed status_64 block.
[ ... ]
> @@ -2556,9 +2698,15 @@ static int bcmgenet_rx_poll(struct napi_struct *napi, int budget)
> {
> struct bcmgenet_rx_ring *ring = container_of(napi,
> struct bcmgenet_rx_ring, napi);
> + struct bcmgenet_priv *priv = ring->priv;
> struct dim_sample dim_sample = {};
> unsigned int work_done;
>
> + /* Reclaim completed XDP TX frames (ring 16 has no interrupt) */
> + if (priv->xdp_prog)
> + bcmgenet_tx_reclaim(priv->dev,
> + &priv->xdp_tx_ring, false);
> +
What happens to in-flight frames if the XDP program is detached at runtime
and priv->xdp_prog becomes NULL? Will this permanently lose track of the
page pool buffers and DMA mappings since the NAPI poll stops reclaiming
them?
Additionally, if the interface is the target of an XDP_REDIRECT from another
device, a local XDP program is not required and priv->xdp_prog could be
NULL. Does this mean the transmitted frames for redirected traffic will
never be reclaimed, eventually filling the TX ring?
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net-next v2] net: macb: fix use of at91_default_usrio without CONFIG_OF
From: patchwork-bot+netdevbpf @ 2026-04-02 3:30 UTC (permalink / raw)
To: Conor Dooley
Cc: netdev, conor.dooley, lkp, jiawenwu, andrew+netdev, davem,
edumazet, kuba, pabeni, nicolas.ferre, claudiu.beznea, devicetree,
linux-kernel, linux-riscv
In-Reply-To: <20260331-enroll-sensation-50901318a419@spud>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 31 Mar 2026 14:10:21 +0100 you wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> If CONFIG_OF is not enabled, at91_default_usrio is used undeclared in
> gem_default_config. Move at91_default_usrio back above the CONFIG_OF
> section where macb_default_usrio used to be, so that it is unconditionally
> defined and defined prior to any of the users.
>
> [...]
Here is the summary with links:
- [net-next,v2] net: macb: fix use of at91_default_usrio without CONFIG_OF
https://git.kernel.org/netdev/net-next/c/cee10a01e286
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 net-next v2 4/4] net: dsa: initial support for MT7628 embedded switch
From: Jakub Kicinski @ 2026-04-02 3:31 UTC (permalink / raw)
To: joey
Cc: Jakub Kicinski, netdev, horms, pabeni, edumazet, davem, olteanv,
andrew, devicetree, robh, krzk+dt, conor+dt
In-Reply-To: <20260330184017.766200-5-joey@tinyisr.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: dsa: initial support for MT7628 embedded switch
Add support for the MT7628 embedded switch.
The switch has 5 built-in 100Mbps user ports (ports 0-4) and one 1Gbps
port that is internally attached to the SoCs CPU MAC and serves as the
CPU port.
> diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
> --- a/drivers/net/dsa/Kconfig
> +++ b/drivers/net/dsa/Kconfig
> @@ -28,6 +28,14 @@ source "drivers/net/dsa/hirschmann/Kconfig"
>
> source "drivers/net/dsa/lantiq/Kconfig"
>
> +config NET_DSA_MT7628
> + tristate "MediaTek MT7628 Embedded Ethernet switch support"
> + select NET_DSA_TAG_MT7628
> + select MEDIATEK_FE_SOC_PHY
Does this configuration need to select NET_DSA_TAG_8021Q?
The driver relies on the tag_8021q mechanism for switch port isolation. If
the kernel is configured without NET_DSA_TAG_8021Q, dsa_tag_8021q_register()
will return -EOPNOTSUPP, causing the switch to boot with broken port
isolation.
> + help
> + This enables support for the built-in Ethernet switch found
> + in the MT7628 SoC.
> +
[ ... ]
> diff --git a/drivers/net/dsa/mt7628.c b/drivers/net/dsa/mt7628.c
> --- /dev/null
> +++ b/drivers/net/dsa/mt7628.c
[ ... ]
> +static int mt7628_mii_read(struct mii_bus *bus, int port, int regnum)
> +{
> + struct mt7628_esw *esw = bus->priv;
> + int ret;
> + u32 val;
> +
> + ret = regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,
> + !(val & MT7628_ESW_PCR1_RD_DONE), 10,
> + 5000);
Will this polling logic cause an MDIO timeout?
If RD_DONE is a completion status bit that remains 1 after a successful
operation, this first poll will wait indefinitely for the bit to become 0.
If RD_DONE is an active-low busy bit, the second poll below waits for the
controller to become busy instead of waiting for it to be idle.
> + if (ret)
> + goto out;
> +
> + ret = regmap_write(esw->regmap, MT7628_ESW_REG_PCR0,
> + FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_REG,
> + regnum) |
> + FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_ADDR,
> + port) | MT7628_ESW_PCR0_RD_PHY_CMD);
> + if (ret)
> + goto out;
> +
> + ret = regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,
> + (val & MT7628_ESW_PCR1_RD_DONE), 10,
> + 5000);
> +out:
> + if (ret) {
> + dev_err(&bus->dev, "read failed. MDIO timeout?\n");
> + return ret;
> + }
> + return FIELD_GET(MT7628_ESW_PCR1_RD_DATA, val);
> +}
> +
> +static int mt7628_mii_write(struct mii_bus *bus, int port, int regnum, u16 dat)
> +{
> + struct mt7628_esw *esw = bus->priv;
> + u32 val;
> + int ret;
> +
> + ret = regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,
> + !(val & MT7628_ESW_PCR1_WT_DONE), 10,
> + 5000);
> + if (ret)
> + goto out;
> +
> + ret = regmap_write(esw->regmap, MT7628_ESW_REG_PCR0,
> + FIELD_PREP(MT7628_ESW_PCR0_WT_NWAY_DATA, dat) |
> + FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_REG,
> + regnum) |
> + FIELD_PREP(MT7628_ESW_PCR0_CPU_PHY_ADDR,
> + port) | MT7628_ESW_PCR0_WT_PHY_CMD);
> + if (ret)
> + goto out;
> +
> + ret = regmap_read_poll_timeout(esw->regmap, MT7628_ESW_REG_PCR1, val,
> + (val & MT7628_ESW_PCR1_WT_DONE), 10,
> + 5000);
Is this second poll waiting for the controller to become busy instead of
idle, similar to the read function?
> +out:
> + if (ret) {
> + dev_err(&bus->dev, "write failed. MDIO timeout?\n");
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int mt7628_setup_internal_mdio(struct dsa_switch *ds)
> +{
> + struct mt7628_esw *esw = ds->priv;
> + struct device_node *mdio;
> + struct mii_bus *bus;
> + int ret = 0;
> +
> + mdio = of_get_available_child_by_name(ds->dev->of_node, "mdio");
> +
> + bus = devm_mdiobus_alloc(esw->dev);
Can allocating devm_ resources in the DSA .setup callback cause a sysfs name
collision?
The .setup callback is executed whenever the DSA tree binds. Since devm_
resources are tied to the platform device (esw->dev) rather than the DSA
tree lifecycle, they are not released when the DSA tree unbinds. On a
subsequent re-bind, allocating a new MDIO bus with the same ID will fail
because the original bus was never unregistered.
> + if (!bus) {
> + ret = -ENOMEM;
> + goto out_put_node;
> + }
> +
> + bus->name = "MT7628 internal MDIO bus";
> + snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(ds->dev));
> + bus->priv = esw;
> + bus->read = mt7628_mii_read;
> + bus->write = mt7628_mii_write;
> + bus->parent = esw->dev;
> + if (!mdio) {
> + ds->user_mii_bus = bus;
> + bus->phy_mask = ~ds->phys_mii_mask;
> + }
> +
> + ret = devm_of_mdiobus_register(esw->dev, bus, mdio);
> +
> +out_put_node:
> + of_node_put(mdio);
> + return ret;
> +}
[ ... ]
> +static int mt7628_setup(struct dsa_switch *ds)
> +{
> + struct mt7628_esw *esw = ds->priv;
> + int ret;
> +
> + reset_control_reset(esw->rst_esw);
> + usleep_range(1000, 2000);
> + reset_control_reset(esw->rst_ephy);
> + usleep_range(1000, 2000);
> + /*
> + * all MMIO reads hang if esw is not out of reset
> + * ephy needs extra time to get out of reset or it ends up misconfigured
> + */
> + mt7628_switch_init(ds);
> + rtnl_lock();
> + dsa_tag_8021q_register(ds, htons(ETH_P_8021Q));
Is it safe to ignore the return value of dsa_tag_8021q_register()?
If the registration fails, the core function frees the allocated context but
does not clear ds->tag_8021q_ctx. The setup will continue, and the DSA core
will later dereference the non-NULL ds->tag_8021q_ctx, leading to a
use-after-free crash.
Also, if mt7628_setup_internal_mdio() below fails, the setup returns an
error without unregistering the tagger, which leaks the tag_8021q context.
> + rtnl_unlock();
> +
> + ret = mt7628_setup_internal_mdio(ds);
> + return ret;
> +}
[ ... ]
> +static struct dsa_switch_ops mt7628_switch_ops = {
> + .get_tag_protocol = mt7628_get_tag_proto,
> + .setup = mt7628_setup,
Does this switch ops structure need a .teardown callback?
Without calling dsa_tag_8021q_unregister() in a .teardown callback, the
tag_8021q context memory allocated during .setup will be permanently leaked
whenever the DSA switch is unbound or the module is removed.
> + .port_enable = mt7628_port_enable,
> + .port_disable = mt7628_port_disable,
> + .phylink_get_caps = mt7628_phylink_get_caps,
> + .tag_8021q_vlan_add = mt7628_dsa_8021q_vlan_add,
> + .tag_8021q_vlan_del = mt7628_dsa_8021q_vlan_del,
> +};
--
pw-bot: cr
^ permalink raw reply
* [PATCH net v1] net: skb: fix cross-cache free of KFENCE-allocated skb head
From: Jiayuan Chen @ 2026-04-02 3:31 UTC (permalink / raw)
To: netdev
Cc: Jiayuan Chen, Antonius, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jason Xing,
Kuniyuki Iwashima, Michal Luczaj, Mina Almasry, Eric Biggers,
Toke Høiland-Jørgensen, Soheil Hassas Yeganeh,
Alexander Duyck, linux-kernel, bpf
SKB_SMALL_HEAD_CACHE_SIZE is intentionally set to a non-power-of-2
value (e.g. 704 on x86_64) to avoid collisions with generic kmalloc
bucket sizes. This ensures that skb_kfree_head() can reliably use
skb_end_offset to distinguish skb heads allocated from
skb_small_head_cache vs. generic kmalloc caches.
However, when KFENCE is enabled, kfence_ksize() returns the exact
requested allocation size instead of the slab bucket size. If a caller
(e.g. bpf_test_init) allocates skb head data via kzalloc() and the
requested size happens to equal SKB_SMALL_HEAD_CACHE_SIZE, then
slab_build_skb() → ksize() returns that exact value. After subtracting
skb_shared_info overhead, skb_end_offset ends up matching
SKB_SMALL_HEAD_HEADROOM, causing skb_kfree_head() to incorrectly free
the object to skb_small_head_cache instead of back to the original
kmalloc cache, resulting in a slab cross-cache free:
kmem_cache_free(skbuff_small_head): Wrong slab cache. Expected
skbuff_small_head but got kmalloc-1k
Fix this by adding an is_kfence_address() check in skb_kfree_head().
When the head is a KFENCE object, we skip the kmem_cache_free() path
and fall through to kfree(), which correctly handles KFENCE objects
via kfence_free(). The check compiles away when CONFIG_KFENCE is
disabled.
Fixes: bf9f1baa279f ("net: add dedicated kmem_cache for typical/small skb->head")
Reported-by: Antonius <antonius@bluedragonsec.com>
Closes: https://lore.kernel.org/netdev/CAK8a0jxC5L5N7hq-DT2_NhUyjBxrPocoiDazzsBk4TGgT1r4-A@mail.gmail.com/
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
net/core/skbuff.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0e217041958a..87cecd40381b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -91,6 +91,7 @@
#include <linux/user_namespace.h>
#include <linux/indirect_call_wrapper.h>
#include <linux/textsearch.h>
+#include <linux/kfence.h>
#include "dev.h"
#include "devmem.h"
@@ -1083,7 +1084,8 @@ static int skb_pp_frag_ref(struct sk_buff *skb)
static void skb_kfree_head(void *head, unsigned int end_offset)
{
- if (end_offset == SKB_SMALL_HEAD_HEADROOM)
+ if (end_offset == SKB_SMALL_HEAD_HEADROOM &&
+ !is_kfence_address(head))
kmem_cache_free(net_hotdata.skb_small_head_cache, head);
else
kfree(head);
--
2.43.0
^ 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