* Re: [PATCH net v2] net: txgbe: fix RTNL assertion warning when remove module
From: Russell King (Oracle) @ 2026-04-07 10:44 UTC (permalink / raw)
To: Jiawen Wu
Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Mengyuan Lou, stable
In-Reply-To: <8B47A5872884147D+20260407094041.4646-1-jiawenwu@trustnetic.com>
On Tue, Apr 07, 2026 at 05:40:41PM +0800, Jiawen Wu wrote:
> For the copper NIC with external PHY, the driver called
> phylink_connect_phy() during probe and phylink_disconnect_phy() during
> remove. It caused an RTNL assertion warning in phylink_disconnect_phy()
> upon module remove.
>
> To fix this, add rtnl_lock() and rtnl_unlock() around the
> phylink_disconnect_phy() in remove function.
>
> ------------[ cut here ]------------
> RTNL: assertion failed at drivers/net/phy/phylink.c (2351)
> WARNING: drivers/net/phy/phylink.c:2351 at
> phylink_disconnect_phy+0xd8/0xf0 [phylink], CPU#0: rmmod/4464
> Modules linked in: ...
> CPU: 0 UID: 0 PID: 4464 Comm: rmmod Kdump: loaded Not tainted 7.0.0-rc4+
> Hardware name: Micro-Star International Co., Ltd. MS-7E16/X670E GAMING
> PLUS WIFI (MS-7E16), BIOS 1.90 12/31/2024
> RIP: 0010:phylink_disconnect_phy+0xe4/0xf0 [phylink]
> Code: 5b 41 5c 41 5d 41 5e 41 5f 5d 31 c0 31 d2 31 f6 31 ff e9 3a 38 8f e7
> 48 8d 3d 48 87 e2 ff ba 2f 09 00 00 48 c7 c6 c1 22 24 c0 <67> 48 0f b9 3a
> e9 34 ff ff ff 66 90 90 90 90 90 90 90 90 90 90 90
> RSP: 0018:ffffce7288363ac0 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff89654b2a1a00 RCX: 0000000000000000
> RDX: 000000000000092f RSI: ffffffffc02422c1 RDI: ffffffffc0239020
> RBP: ffffce7288363ae8 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: ffff8964c4022000
> R13: ffff89654fce3028 R14: ffff89654ebb4000 R15: ffffffffc0226348
> FS: 0000795e80d93780(0000) GS:ffff896c52857000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00005b528b592000 CR3: 0000000170d0f000 CR4: 0000000000f50ef0
> PKRU: 55555554
> Call Trace:
> <TASK>
> txgbe_remove_phy+0xbb/0xd0 [txgbe]
> txgbe_remove+0x4c/0xb0 [txgbe]
> pci_device_remove+0x41/0xb0
> device_remove+0x43/0x80
> device_release_driver_internal+0x206/0x270
> driver_detach+0x4a/0xa0
> bus_remove_driver+0x83/0x120
> driver_unregister+0x2f/0x60
> pci_unregister_driver+0x40/0x90
> txgbe_driver_exit+0x10/0x850 [txgbe]
> __do_sys_delete_module.isra.0+0x1c3/0x2f0
> __x64_sys_delete_module+0x12/0x20
> x64_sys_call+0x20c3/0x2390
> do_syscall_64+0x11c/0x1500
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? do_syscall_64+0x15a/0x1500
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? do_fault+0x312/0x580
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __handle_mm_fault+0x9d5/0x1040
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? count_memcg_events+0x101/0x1d0
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? handle_mm_fault+0x1e8/0x2f0
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? do_user_addr_fault+0x2f8/0x820
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? irqentry_exit+0xb2/0x600
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? exc_page_fault+0x92/0x1c0
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Fixes: 02b2a6f91b90 ("net: txgbe: support copper NIC with external PHY")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
Thanks!
--
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] net: rtnetlink: zero ifla_vf_broadcast to avoid stack infoleak in rtnl_fill_vfinfo
From: Greg KH @ 2026-04-07 10:42 UTC (permalink / raw)
To: Kai Zen
Cc: netdev, edwin.peer, Eric Dumazet, davem, kuba, pabeni, horms,
stable, security
In-Reply-To: <CALynFi7k1Z7Vgr4p2=KH2-uWVntBRE5R+8uP=cds9_ihGqzOdQ@mail.gmail.com>
On Tue, Apr 07, 2026 at 01:21:57PM +0300, Kai Zen wrote:
> rtnl_fill_vfinfo() declares struct ifla_vf_broadcast on the stack
> without initialisation:
>
> struct ifla_vf_broadcast vf_broadcast;
>
> The struct contains a single fixed 32-byte field:
>
> /* include/uapi/linux/if_link.h */
> struct ifla_vf_broadcast {
> __u8 broadcast[32];
> };
>
> The function then copies dev->broadcast into it using dev->addr_len
> as the length:
>
> memcpy(vf_broadcast.broadcast, dev->broadcast, dev->addr_len);
>
> On Ethernet devices (the overwhelming majority of SR-IOV NICs)
> dev->addr_len is 6, so only the first 6 bytes of broadcast[] are
> written. The remaining 26 bytes retain whatever was previously on
> the kernel stack. The full struct is then handed to userspace via:
>
> nla_put(skb, IFLA_VF_BROADCAST,
> sizeof(vf_broadcast), &vf_broadcast)
>
> leaking up to 26 bytes of uninitialised kernel stack per VF per
> RTM_GETLINK request, repeatable.
>
> The other vf_* structs in the same function are explicitly zeroed
> for exactly this reason - see the memset() calls for ivi,
> vf_vlan_info, node_guid and port_guid a few lines above.
> vf_broadcast was simply missed when it was added.
>
> The pattern used elsewhere in this file for the regular IFLA_BROADCAST
> attribute also avoids the issue by sending only dev->addr_len bytes
> rather than a fixed-size struct, but for IFLA_VF_BROADCAST the wire
> format is the fixed 32-byte struct, so the right fix is to zero the
> struct before the partial memcpy.
>
> Reachability and impact
> -----------------------
>
> The leak is reachable by any unprivileged local process. AF_NETLINK
> with NETLINK_ROUTE requires no capabilities. The only environmental
> requirement is that the host has at least one SR-IOV-capable
> interface present (a parent device with VFs), which is the common
> case for cloud, datacenter and HPC hosts.
>
> Trigger: send RTM_GETLINK with an IFLA_EXT_MASK attribute whose
> value has the RTEXT_FILTER_VF bit set. The kernel will then walk
> each VF and emit IFLA_VFINFO_LIST, including IFLA_VF_BROADCAST,
> which carries the 26 bytes of uninitialised stack per VF.
>
> Stack residue at this call site can include return addresses
> (useful as a KASLR / function-pointer disclosure primitive) and
> transient sensitive data left over by whatever ran on the same
> kernel stack just prior. KASAN with stack instrumentation, or
> KMSAN, will flag the nla_put() when reproduced.
>
> Reproducer (unprivileged):
>
> import socket, struct
> IFLA_EXT_MASK = 29
> RTEXT_FILTER_VF = 1
> s = socket.socket(socket.AF_NETLINK, socket.SOCK_RAW,
> socket.NETLINK_ROUTE)
> s.bind((0, 0))
> hdr = struct.pack('=IHHII', 0, 18, 0x301, 0, 0)
> ifi = struct.pack('=BxHiII', 0, 0, 0, 0, 0)
> attr = (struct.pack('=HH', 8, IFLA_EXT_MASK) +
> struct.pack('=I', RTEXT_FILTER_VF))
> msg = hdr + ifi + attr
> msg = struct.pack('=I', len(msg)) + msg[4:]
> s.send(msg)
> data = s.recv(65536)
> # Parse IFLA_VF_BROADCAST from the response. Bytes 7..32 of the
> # broadcast[] field are uninitialised kernel stack on Ethernet.
>
> Fix
> ---
>
> Zero the on-stack struct before the partial memcpy, matching the
> existing pattern used for the other vf_* structs in the same
> function.
>
> Reported-by: Kai Aizen <kai.aizen.dev@gmail.com>
> Signed-off-by: Kai Aizen <kai.aizen.dev@gmail.com>
> ---
>
> Note for reviewers: this is v1. I have not yet identified the
> exact introducing commit for the Fixes: tag and would appreciate
> a pointer, or I will resend as v2 once I have run git blame on a
> local checkout. The bug is present at least as far back as the
> introduction of struct ifla_vf_broadcast in net-next.
>
> net/core/rtnetlink.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1572,6 +1572,7 @@ static noinline_for_stack int
> rtnl_fill_vfinfo(struct sk_buff *skb,
> port_guid.vf = ivi.vf;
>
> memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac));
> + memset(&vf_broadcast, 0, sizeof(vf_broadcast));
> memcpy(vf_broadcast.broadcast, dev->broadcast, dev->addr_len);
> vf_vlan.vlan = ivi.vlan;
> vf_vlan.qos = ivi.qos;
Nice catch, but your patch is corrupted and can't be applied as the tabs
were converted to spaces and the commit is line-wrapped :(
With that fixed, feel free to resubmit and add:
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net v2] ipvs: fix MTU check for GSO packets in tunnel mode
From: kernel test robot @ 2026-04-07 10:38 UTC (permalink / raw)
To: Yingnan Zhang, horms, ja
Cc: llvm, oe-kbuild-all, pablo, fw, phil, davem, edumazet, kuba,
pabeni, netdev, lvs-devel, netfilter-devel, coreteam,
linux-kernel, Yingnan Zhang
In-Reply-To: <tencent_CA2C1C219C99D315086BE55E8654AF7E6009@qq.com>
Hi Yingnan,
kernel test robot noticed the following build errors:
[auto build test ERROR on net/main]
url: https://github.com/intel-lab-lkp/linux/commits/Yingnan-Zhang/ipvs-fix-MTU-check-for-GSO-packets-in-tunnel-mode/20260407-141549
base: net/main
patch link: https://lore.kernel.org/r/tencent_CA2C1C219C99D315086BE55E8654AF7E6009%40qq.com
patch subject: [PATCH net v2] ipvs: fix MTU check for GSO packets in tunnel mode
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260407/202604071825.Sh4Y1fMi-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260407/202604071825.Sh4Y1fMi-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604071825.Sh4Y1fMi-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> net/netfilter/ipvs/ip_vs_xmit.c:115:2: warning: non-void function does not return a value in all control paths [-Wreturn-type]
115 | } else if (skb->len > mtu &&
| ^
>> net/netfilter/ipvs/ip_vs_xmit.c:115:4: error: expected identifier or '('
115 | } else if (skb->len > mtu &&
| ^
net/netfilter/ipvs/ip_vs_xmit.c:119:2: error: expected identifier or '('
119 | return false;
| ^
>> net/netfilter/ipvs/ip_vs_xmit.c:120:1: error: extraneous closing brace ('}')
120 | }
| ^
1 warning and 3 errors generated.
vim +115 net/netfilter/ipvs/ip_vs_xmit.c
104
105 static inline bool
106 __mtu_check_toobig_v6(const struct sk_buff *skb, u32 mtu)
107 {
108 if (IP6CB(skb)->frag_max_size) {
109 /* frag_max_size tell us that, this packet have been
110 * defragmented by netfilter IPv6 conntrack module.
111 */
112 if (IP6CB(skb)->frag_max_size > mtu)
113 return true; /* largest fragment violate MTU */
114 }
> 115 } else if (skb->len > mtu &&
116 !(skb_is_gso(skb) && skb_gso_validate_network_len(skb, mtu))) {
117 return true; /* Packet size violate MTU size */
118 }
119 return false;
> 120 }
121
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH 5/6] arm64: dts: qcom: milos: Add IPA node
From: Konrad Dybcio @ 2026-04-07 10:30 UTC (permalink / raw)
To: Luca Weiss, Alex Elder, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Alexander Koskovich
Cc: ~postmarketos/upstreaming, phone-devel, netdev, linux-kernel,
linux-arm-msm, devicetree
In-Reply-To: <20260403-milos-ipa-v1-5-01e9e4e03d3e@fairphone.com>
On 4/3/26 6:43 PM, Luca Weiss wrote:
> Add the description of the IPA block in the Milos SoC.
>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
> + iommus = <&apps_smmu 0x4a0 0x0>,
> + <&apps_smmu 0x4a2 0x0>;
P.S.
I don't know what's the scope of upstream IPA today, but it seems like
there's two additional SIDs: 0x4a1 attached to an "ipa_smmu_wlan"
subnode and another one (0x4a4) called ipa_smmu_11ad, perhaps for
some tighter integration with ath1xk_ahb?
But again, I don't know much.
Konrad
^ permalink raw reply
* Re: [PATCH net] ipv6: add READ_ONCE() annotations to fib6_metrics reader paths
From: Paolo Abeni @ 2026-04-07 10:27 UTC (permalink / raw)
To: Hangbin Liu, David S. Miller, David Ahern, Eric Dumazet,
Jakub Kicinski, Simon Horman
Cc: David Ahern, netdev, linux-kernel, Jiayuan Chen
In-Reply-To: <20260403-fib6_metric_read_once-v1-1-45de44912b24@gmail.com>
On 4/3/26 6:16 AM, Hangbin Liu wrote:
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index 9f8b6814a96a..2dfb04fab5da 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -594,7 +594,9 @@ void fib6_update_sernum_stub(struct net *net, struct fib6_info *f6i);
> void fib6_metric_set(struct fib6_info *f6i, int metric, u32 val);
> static inline bool fib6_metric_locked(struct fib6_info *f6i, int metric)
> {
> - return !!(f6i->fib6_metrics->metrics[RTAX_LOCK - 1] & (1 << metric));
> + struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
> +
> + return !!(m->metrics[RTAX_LOCK - 1] & (1 << metric));
Sashiko notes that here you may want to add an additional READ_ONCE() on
m->metrics[RTAX_LOCK - 1], which in turn looks like more a
follow-up/separate change than a change specific to this patch
> }
> void fib6_info_hw_flags_set(struct net *net, struct fib6_info *f6i,
> bool offload, bool trap, bool offload_failed);
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index dd26657b6a4a..3c96580c2a03 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -1144,9 +1144,11 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
> iter->fib6_flags &= ~RTF_PREFIX_RT;
> }
>
> - if (rt->fib6_pmtu)
> + u32 pmtu = READ_ONCE(rt->fib6_pmtu);
Here the READ_ONCE() on rt->metrics is still missing.
> @@ -1635,11 +1635,12 @@ __rt6_find_exception_rcu(struct rt6_exception_bucket **bucket,
> static unsigned int fib6_mtu(const struct fib6_result *res)
> {
> const struct fib6_nh *nh = res->nh;
> + struct dst_metrics *m;
> unsigned int mtu;
>
> - if (res->f6i->fib6_pmtu) {
> - mtu = res->f6i->fib6_pmtu;
> - } else {
> + m = READ_ONCE(res->f6i->fib6_metrics);
> + mtu = READ_ONCE(m->metrics[RTAX_MTU - 1]);
After this patch there will be a single usage of the `fib6_pmtu` macro.
I think it would be better to entirely drop it and replace with an helper:
static inline unsigned int fib6_mtu(const struct fib6_info *f6i)
{
const struct dst_metrics *m = READ_ONCE(f6i->fib6_metrics);
return READ_ONCE(m->metrics[RTAX_MTU - 1]);
}
/P
^ permalink raw reply
* Re: [PATCH net] net: rtnetlink: zero ifla_vf_broadcast to avoid stack infoleak in rtnl_fill_vfinfo
From: Eric Dumazet @ 2026-04-07 10:26 UTC (permalink / raw)
To: Kai Zen; +Cc: netdev, edwin.peer, davem, kuba, pabeni, horms, stable, security
In-Reply-To: <CALynFi7k1Z7Vgr4p2=KH2-uWVntBRE5R+8uP=cds9_ihGqzOdQ@mail.gmail.com>
On Tue, Apr 7, 2026 at 3:22 AM Kai Zen <kai.aizen.dev@gmail.com> wrote:
>
> rtnl_fill_vfinfo() declares struct ifla_vf_broadcast on the stack
> without initialisation:
>
> struct ifla_vf_broadcast vf_broadcast;
>
> The struct contains a single fixed 32-byte field:
>
> /* include/uapi/linux/if_link.h */
> struct ifla_vf_broadcast {
> __u8 broadcast[32];
> };
>
> The function then copies dev->broadcast into it using dev->addr_len
> as the length:
>
> memcpy(vf_broadcast.broadcast, dev->broadcast, dev->addr_len);
>
> On Ethernet devices (the overwhelming majority of SR-IOV NICs)
> dev->addr_len is 6, so only the first 6 bytes of broadcast[] are
> written. The remaining 26 bytes retain whatever was previously on
> the kernel stack. The full struct is then handed to userspace via:
>
> nla_put(skb, IFLA_VF_BROADCAST,
> sizeof(vf_broadcast), &vf_broadcast)
>
> leaking up to 26 bytes of uninitialised kernel stack per VF per
> RTM_GETLINK request, repeatable.
>
> The other vf_* structs in the same function are explicitly zeroed
> for exactly this reason - see the memset() calls for ivi,
> vf_vlan_info, node_guid and port_guid a few lines above.
> vf_broadcast was simply missed when it was added.
>
> The pattern used elsewhere in this file for the regular IFLA_BROADCAST
> attribute also avoids the issue by sending only dev->addr_len bytes
> rather than a fixed-size struct, but for IFLA_VF_BROADCAST the wire
> format is the fixed 32-byte struct, so the right fix is to zero the
> struct before the partial memcpy.
>
> Reachability and impact
> -----------------------
>
> The leak is reachable by any unprivileged local process. AF_NETLINK
> with NETLINK_ROUTE requires no capabilities. The only environmental
> requirement is that the host has at least one SR-IOV-capable
> interface present (a parent device with VFs), which is the common
> case for cloud, datacenter and HPC hosts.
>
> Trigger: send RTM_GETLINK with an IFLA_EXT_MASK attribute whose
> value has the RTEXT_FILTER_VF bit set. The kernel will then walk
> each VF and emit IFLA_VFINFO_LIST, including IFLA_VF_BROADCAST,
> which carries the 26 bytes of uninitialised stack per VF.
>
> Stack residue at this call site can include return addresses
> (useful as a KASLR / function-pointer disclosure primitive) and
> transient sensitive data left over by whatever ran on the same
> kernel stack just prior. KASAN with stack instrumentation, or
> KMSAN, will flag the nla_put() when reproduced.
>
> Reproducer (unprivileged):
>
> import socket, struct
> IFLA_EXT_MASK = 29
> RTEXT_FILTER_VF = 1
> s = socket.socket(socket.AF_NETLINK, socket.SOCK_RAW,
> socket.NETLINK_ROUTE)
> s.bind((0, 0))
> hdr = struct.pack('=IHHII', 0, 18, 0x301, 0, 0)
> ifi = struct.pack('=BxHiII', 0, 0, 0, 0, 0)
> attr = (struct.pack('=HH', 8, IFLA_EXT_MASK) +
> struct.pack('=I', RTEXT_FILTER_VF))
> msg = hdr + ifi + attr
> msg = struct.pack('=I', len(msg)) + msg[4:]
> s.send(msg)
> data = s.recv(65536)
> # Parse IFLA_VF_BROADCAST from the response. Bytes 7..32 of the
> # broadcast[] field are uninitialised kernel stack on Ethernet.
>
> Fix
> ---
>
> Zero the on-stack struct before the partial memcpy, matching the
> existing pattern used for the other vf_* structs in the same
> function.
>
> Reported-by: Kai Aizen <kai.aizen.dev@gmail.com>
> Signed-off-by: Kai Aizen <kai.aizen.dev@gmail.com>
> ---
>
> Note for reviewers: this is v1. I have not yet identified the
> exact introducing commit for the Fixes: tag and would appreciate
> a pointer, or I will resend as v2 once I have run git blame on a
> local checkout. The bug is present at least as far back as the
> introduction of struct ifla_vf_broadcast in net-next.
Fixes: 75345f888f70 ("ipoib: show VF broadcast address")
>
> net/core/rtnetlink.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1572,6 +1572,7 @@ static noinline_for_stack int
> rtnl_fill_vfinfo(struct sk_buff *skb,
> port_guid.vf = ivi.vf;
>
> memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac));
> + memset(&vf_broadcast, 0, sizeof(vf_broadcast));
> memcpy(vf_broadcast.broadcast, dev->broadcast, dev->addr_len);
> vf_vlan.vlan = ivi.vlan;
> vf_vlan.qos = ivi.qos;
> --
> 2.43.0
^ permalink raw reply
* [PATCH net] net: rtnetlink: zero ifla_vf_broadcast to avoid stack infoleak in rtnl_fill_vfinfo
From: Kai Zen @ 2026-04-07 10:21 UTC (permalink / raw)
To: netdev
Cc: edwin.peer, Eric Dumazet, davem, kuba, pabeni, horms, stable,
security
rtnl_fill_vfinfo() declares struct ifla_vf_broadcast on the stack
without initialisation:
struct ifla_vf_broadcast vf_broadcast;
The struct contains a single fixed 32-byte field:
/* include/uapi/linux/if_link.h */
struct ifla_vf_broadcast {
__u8 broadcast[32];
};
The function then copies dev->broadcast into it using dev->addr_len
as the length:
memcpy(vf_broadcast.broadcast, dev->broadcast, dev->addr_len);
On Ethernet devices (the overwhelming majority of SR-IOV NICs)
dev->addr_len is 6, so only the first 6 bytes of broadcast[] are
written. The remaining 26 bytes retain whatever was previously on
the kernel stack. The full struct is then handed to userspace via:
nla_put(skb, IFLA_VF_BROADCAST,
sizeof(vf_broadcast), &vf_broadcast)
leaking up to 26 bytes of uninitialised kernel stack per VF per
RTM_GETLINK request, repeatable.
The other vf_* structs in the same function are explicitly zeroed
for exactly this reason - see the memset() calls for ivi,
vf_vlan_info, node_guid and port_guid a few lines above.
vf_broadcast was simply missed when it was added.
The pattern used elsewhere in this file for the regular IFLA_BROADCAST
attribute also avoids the issue by sending only dev->addr_len bytes
rather than a fixed-size struct, but for IFLA_VF_BROADCAST the wire
format is the fixed 32-byte struct, so the right fix is to zero the
struct before the partial memcpy.
Reachability and impact
-----------------------
The leak is reachable by any unprivileged local process. AF_NETLINK
with NETLINK_ROUTE requires no capabilities. The only environmental
requirement is that the host has at least one SR-IOV-capable
interface present (a parent device with VFs), which is the common
case for cloud, datacenter and HPC hosts.
Trigger: send RTM_GETLINK with an IFLA_EXT_MASK attribute whose
value has the RTEXT_FILTER_VF bit set. The kernel will then walk
each VF and emit IFLA_VFINFO_LIST, including IFLA_VF_BROADCAST,
which carries the 26 bytes of uninitialised stack per VF.
Stack residue at this call site can include return addresses
(useful as a KASLR / function-pointer disclosure primitive) and
transient sensitive data left over by whatever ran on the same
kernel stack just prior. KASAN with stack instrumentation, or
KMSAN, will flag the nla_put() when reproduced.
Reproducer (unprivileged):
import socket, struct
IFLA_EXT_MASK = 29
RTEXT_FILTER_VF = 1
s = socket.socket(socket.AF_NETLINK, socket.SOCK_RAW,
socket.NETLINK_ROUTE)
s.bind((0, 0))
hdr = struct.pack('=IHHII', 0, 18, 0x301, 0, 0)
ifi = struct.pack('=BxHiII', 0, 0, 0, 0, 0)
attr = (struct.pack('=HH', 8, IFLA_EXT_MASK) +
struct.pack('=I', RTEXT_FILTER_VF))
msg = hdr + ifi + attr
msg = struct.pack('=I', len(msg)) + msg[4:]
s.send(msg)
data = s.recv(65536)
# Parse IFLA_VF_BROADCAST from the response. Bytes 7..32 of the
# broadcast[] field are uninitialised kernel stack on Ethernet.
Fix
---
Zero the on-stack struct before the partial memcpy, matching the
existing pattern used for the other vf_* structs in the same
function.
Reported-by: Kai Aizen <kai.aizen.dev@gmail.com>
Signed-off-by: Kai Aizen <kai.aizen.dev@gmail.com>
---
Note for reviewers: this is v1. I have not yet identified the
exact introducing commit for the Fixes: tag and would appreciate
a pointer, or I will resend as v2 once I have run git blame on a
local checkout. The bug is present at least as far back as the
introduction of struct ifla_vf_broadcast in net-next.
net/core/rtnetlink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1572,6 +1572,7 @@ static noinline_for_stack int
rtnl_fill_vfinfo(struct sk_buff *skb,
port_guid.vf = ivi.vf;
memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac));
+ memset(&vf_broadcast, 0, sizeof(vf_broadcast));
memcpy(vf_broadcast.broadcast, dev->broadcast, dev->addr_len);
vf_vlan.vlan = ivi.vlan;
vf_vlan.qos = ivi.qos;
--
2.43.0
^ permalink raw reply
* [PATCH net-next v2] selftests: forwarding: lib: rewrite processing of command line arguments
From: Ioana Ciornei @ 2026-04-07 10:20 UTC (permalink / raw)
To: netdev
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, linux-kernel, petrm, willemb,
linux-kselftest, Shuah Khan, Hangbin Liu
The piece of code which processes the command line arguments and
populates NETIFS based on them is really unobvious. Rewrite it so that
the intention is clear and the code is easy to follow.
Suggested-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v2:
- added back 'count=0'
tools/testing/selftests/net/forwarding/lib.sh | 21 ++++++++++---------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index d8cc4c64148d..d2bdbff68075 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -467,17 +467,18 @@ if [ "${DRIVER_TEST_CONFORMANT}" = "yes" ]; then
TARGETS[$remote_netif]="$REMOTE_TYPE:$REMOTE_ARGS"
else
count=0
+ # Prime NETIFS from the command line, but retain if none given.
+ if [[ $# -gt 0 ]]; then
+ unset NETIFS
+ declare -A NETIFS
- while [[ $# -gt 0 ]]; do
- if [[ "$count" -eq "0" ]]; then
- unset NETIFS
- declare -A NETIFS
- fi
- count=$((count + 1))
- NETIFS[p$count]="$1"
- TARGETS[$1]="local:"
- shift
- done
+ while [[ $# -gt 0 ]]; do
+ count=$((count + 1))
+ NETIFS[p$count]="$1"
+ TARGETS[$1]="local:"
+ shift
+ done
+ fi
fi
##############################################################################
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net-next v8] selftests: net: add tests for PPP
From: patchwork-bot+netdevbpf @ 2026-04-07 10:20 UTC (permalink / raw)
To: Qingfang Deng
Cc: linux-ppp, shuah, davem, edumazet, kuba, pabeni, horms, bigeasy,
fmaurer, petrm, linux-kernel, linux-kselftest, netdev, paulus,
dianne, jaco, carlsonj, andrew
In-Reply-To: <20260403034908.30017-1-qingfang.deng@linux.dev>
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Fri, 3 Apr 2026 11:48:47 +0800 you wrote:
> Add ping and iperf3 tests for ppp_async.c and pppoe.c.
>
> Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
> ---
> v8:
> - locate pppoe.so and pass it to pppoe-server
> - start socat as a syslog listener and dump the log messages if the test fails
> https://lore.kernel.org/netdev/20260330035604.133073-1-dqfext@gmail.com/
>
> [...]
Here is the summary with links:
- [net-next,v8] selftests: net: add tests for PPP
https://git.kernel.org/netdev/net-next/c/dfecb0c5af3b
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] net: dropreason: add SKB_DROP_REASON_{BROAD,MULTI}CAST_BACKLOG
From: Eric Dumazet @ 2026-04-07 10:09 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S . Miller, Paolo Abeni, Simon Horman, Kuniyuki Iwashima,
Andrew Lunn, netdev, eric.dumazet
In-Reply-To: <20260406180254.5221a267@kernel.org>
On Mon, Apr 6, 2026 at 6:02 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 3 Apr 2026 02:24:43 +0000 Eric Dumazet wrote:
> > ipvlan and macvlan use queues to process broadcast or multicast packets
> > from a work queue.
> >
> > Under attack these queues can drop packets.
> >
> > Add BROADCAST_BACKLOG drop_reason for macvlan broadcast queue.
> >
> > Add MULTICAST_BACKLOG drop_reason for ipvlan multicast queue.
>
> What distinction are you trying to communicate by using BROADCAST
> vs MULTICAST? Funny naming of the functions aside I believe that
> in both cases we're basically dealing with multicast traffic.
> Reading the code it seems like macvlan has some optimization where
> it delivers the frame inline if the multicast group is not very
> popular. And if it is popular it schedules a work (and calls that
> broadcast). ipvlan doesn't have the inline filter and calls the
> deliver via work multicast.
>
> tl;dr I think the drivers do the same thing just call their functions
> broadcast vs multicast. Having two drop reasons here is actively
> misleading?
My intent was to differentiate ipvlan and macvlan reasons, some
deployments use both.
I can rename the reasons if you think this is better.
IPVLAN_MULTICAST_BACKLOG
MACVLAN_BROADCAST_BACKLOG
^ permalink raw reply
* Re: [PATCH net-next v8] selftests: net: add tests for PPP
From: Paolo Abeni @ 2026-04-07 10:08 UTC (permalink / raw)
To: Qingfang Deng, linux-ppp, Shuah Khan, David S. Miller,
Eric Dumazet, Jakub Kicinski, Simon Horman,
Sebastian Andrzej Siewior, Felix Maurer, Petr Machata,
linux-kernel, linux-kselftest, netdev
Cc: Paul Mackerras, Dianne Skoll, Jaco Kroon, James Carlson,
Andrew Lunn
In-Reply-To: <20260403034908.30017-1-qingfang.deng@linux.dev>
On 4/3/26 5:48 AM, Qingfang Deng wrote:
> diff --git a/tools/testing/selftests/net/ppp/pppoe.sh b/tools/testing/selftests/net/ppp/pppoe.sh
> new file mode 100755
> index 000000000000..f67b51df7490
> --- /dev/null
> +++ b/tools/testing/selftests/net/ppp/pppoe.sh
> @@ -0,0 +1,65 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +source ppp_common.sh
> +
> +VETH_SERVER="veth-server"
> +VETH_CLIENT="veth-client"
> +PPPOE_LOG=$(mktemp /tmp/pppoe.XXXXXX)
> +
> +# shellcheck disable=SC2329
> +cleanup() {
> + cleanup_all_ns
> + [ -n "$SOCAT_PID" ] && kill_process "$SOCAT_PID"
> + rm -f "$PPPOE_LOG"
> +}
> +
> +trap cleanup EXIT
> +
> +require_command pppoe-server
> +ppp_common_init
> +modprobe -q pppoe
> +
> +# Try to locate pppoe.so plugin
> +PPPOE_PLUGIN=$(find /usr/{lib,lib64,lib32}/pppd/ -name pppoe.so -type f -print -quit)
Minor nit: here stderr could be redirected to /dev/null, since the
command will likely emit at least an error message on most systems.
Could be a follow-up or added if/when you will expand the self-tests.
/P
^ permalink raw reply
* Re: [PATCH v10 net-next 3/6] devlink: Implement devlink param multi attribute nested data values
From: Paolo Abeni @ 2026-04-07 9:58 UTC (permalink / raw)
To: Ratheesh Kannoth, netdev, linux-kernel, linux-rdma
Cc: sgoutham, andrew+netdev, davem, edumazet, kuba, donald.hunter,
horms, jiri, chuck.lever, matttbe, cjubran, saeedm, leon, tariqt,
mbloch, dtatulea
In-Reply-To: <20260403025533.6250-4-rkannoth@marvell.com>
On 4/3/26 4:55 AM, Ratheesh Kannoth wrote:
> From: Saeed Mahameed <saeedm@nvidia.com>
>
> Devlink param value attribute is not defined since devlink is handling
> the value validating and parsing internally, this allows us to implement
> multi attribute values without breaking any policies.
>
> Devlink param multi-attribute values are considered to be dynamically
> sized arrays of u64 values, by introducing a new devlink param type
> DEVLINK_PARAM_TYPE_U64_ARRAY, driver and user space can set a variable
> count of u32 values into the DEVLINK_ATTR_PARAM_VALUE_DATA attribute.
>
> Implement get/set parsing and add to the internal value structure passed
> to drivers.
>
> This is useful for devices that need to configure a list of values for
> a specific configuration.
>
> example:
> $ devlink dev param show pci/... name multi-value-param
> name multi-value-param type driver-specific
> values:
> cmode permanent value: 0,1,2,3,4,5,6,7
>
> $ devlink dev param set pci/... name multi-value-param \
> value 4,5,6,7,0,1,2,3 cmode permanent
>
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
> Documentation/netlink/specs/devlink.yaml | 4 ++
> include/net/devlink.h | 8 +++
> include/uapi/linux/devlink.h | 1 +
> net/devlink/netlink_gen.c | 2 +
> net/devlink/param.c | 91 +++++++++++++++++++-----
> 5 files changed, 89 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml
> index b495d56b9137..b619de4fe08a 100644
> --- a/Documentation/netlink/specs/devlink.yaml
> +++ b/Documentation/netlink/specs/devlink.yaml
> @@ -226,6 +226,10 @@ definitions:
> value: 10
> -
> name: binary
> + -
> + name: u64-array
> + value: 129
> +
> -
> name: rate-tc-index-max
> type: const
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index 3038af6ec017..3a355fea8189 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -432,6 +432,13 @@ enum devlink_param_type {
> DEVLINK_PARAM_TYPE_U64 = DEVLINK_VAR_ATTR_TYPE_U64,
> DEVLINK_PARAM_TYPE_STRING = DEVLINK_VAR_ATTR_TYPE_STRING,
> DEVLINK_PARAM_TYPE_BOOL = DEVLINK_VAR_ATTR_TYPE_FLAG,
> + DEVLINK_PARAM_TYPE_U64_ARRAY = DEVLINK_VAR_ATTR_TYPE_U64_ARRAY,
> +};
> +
> +#define __DEVLINK_PARAM_MAX_ARRAY_SIZE 32
> +struct devlink_param_u64_array {
> + u64 size;
> + u64 val[__DEVLINK_PARAM_MAX_ARRAY_SIZE];
> };
>
> union devlink_param_value {
> @@ -441,6 +448,7 @@ union devlink_param_value {
> u64 vu64;
> char vstr[__DEVLINK_PARAM_MAX_STRING_VALUE];
> bool vbool;
> + struct devlink_param_u64_array u64arr;
Sashiko as a couple of relevant remarks here, specifically:
---
Does this increase the size of union devlink_param_value from 32 bytes
to over 264 bytes?
Looking at existing functions like devlink_nl_param_value_fill_one() and
devlink_nl_param_value_put(), they take multiple copies of this union by
value. Passing two of these unions by value consumes over 528 bytes of
stack space, and combined in a call chain this pushes nearly 800 bytes
of arguments onto the stack.
Could this create a risk of hitting CONFIG_FRAME_WARN limits deep in
driver notification contexts? Should the signatures of the internal
functions and exported APIs be updated to pass the unions by pointer
instead?
---
> };
>
> struct devlink_param_gset_ctx {
> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> index 7de2d8cc862f..5332223dd6d0 100644
> --- a/include/uapi/linux/devlink.h
> +++ b/include/uapi/linux/devlink.h
> @@ -406,6 +406,7 @@ enum devlink_var_attr_type {
> DEVLINK_VAR_ATTR_TYPE_BINARY,
> __DEVLINK_VAR_ATTR_TYPE_CUSTOM_BASE = 0x80,
> /* Any possible custom types, unrelated to NLA_* values go below */
> + DEVLINK_VAR_ATTR_TYPE_U64_ARRAY,
> };
>
> enum devlink_attr {
> diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c
> index eb35e80e01d1..7aaf462f27ee 100644
> --- a/net/devlink/netlink_gen.c
> +++ b/net/devlink/netlink_gen.c
> @@ -37,6 +37,8 @@ devlink_attr_param_type_validate(const struct nlattr *attr,
> case DEVLINK_VAR_ATTR_TYPE_NUL_STRING:
> fallthrough;
> case DEVLINK_VAR_ATTR_TYPE_BINARY:
> + fallthrough;
> + case DEVLINK_VAR_ATTR_TYPE_U64_ARRAY:
> return 0;
> }
> NL_SET_ERR_MSG_ATTR(extack, attr, "invalid enum value");
> diff --git a/net/devlink/param.c b/net/devlink/param.c
> index cf95268da5b0..2ec85dffd8ac 100644
> --- a/net/devlink/param.c
> +++ b/net/devlink/param.c
> @@ -252,6 +252,14 @@ devlink_nl_param_value_put(struct sk_buff *msg, enum devlink_param_type type,
> return -EMSGSIZE;
> }
> break;
> + case DEVLINK_PARAM_TYPE_U64_ARRAY:
> + if (val.u64arr.size > __DEVLINK_PARAM_MAX_ARRAY_SIZE)
> + return -EMSGSIZE;
> +
> + for (int i = 0; i < val.u64arr.size; i++)
> + if (nla_put_uint(msg, nla_type, val.u64arr.val[i]))
> + return -EMSGSIZE;
> + break;
> }
> return 0;
> }
> @@ -304,56 +312,78 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
> u32 portid, u32 seq, int flags,
> struct netlink_ext_ack *extack)
> {
> - union devlink_param_value default_value[DEVLINK_PARAM_CMODE_MAX + 1];
> - union devlink_param_value param_value[DEVLINK_PARAM_CMODE_MAX + 1];
> bool default_value_set[DEVLINK_PARAM_CMODE_MAX + 1] = {};
> bool param_value_set[DEVLINK_PARAM_CMODE_MAX + 1] = {};
> const struct devlink_param *param = param_item->param;
> - struct devlink_param_gset_ctx ctx;
> + union devlink_param_value *default_value;
> + union devlink_param_value *param_value;
> + struct devlink_param_gset_ctx *ctx;
> struct nlattr *param_values_list;
> struct nlattr *param_attr;
> void *hdr;
> int err;
> int i;
>
> + default_value = kcalloc(DEVLINK_PARAM_CMODE_MAX + 1,
> + sizeof(*default_value), GFP_KERNEL);
> + if (!default_value)
> + return -ENOMEM;
> +
> + param_value = kcalloc(DEVLINK_PARAM_CMODE_MAX + 1,
> + sizeof(*param_value), GFP_KERNEL);
> + if (!param_value) {
> + kfree(default_value);
> + return -ENOMEM;
> + }
> +
> + ctx = kmalloc_obj(*ctx);
> + if (!ctx) {
> + kfree(param_value);
> + kfree(default_value);
> + return -ENOMEM;
> + }
> +
> /* Get value from driver part to driverinit configuration mode */
> for (i = 0; i <= DEVLINK_PARAM_CMODE_MAX; i++) {
> if (!devlink_param_cmode_is_supported(param, i))
> continue;
> if (i == DEVLINK_PARAM_CMODE_DRIVERINIT) {
> - if (param_item->driverinit_value_new_valid)
> + if (param_item->driverinit_value_new_valid) {
> param_value[i] = param_item->driverinit_value_new;
> - else if (param_item->driverinit_value_valid)
> + } else if (param_item->driverinit_value_valid) {
> param_value[i] = param_item->driverinit_value;
> - else
> - return -EOPNOTSUPP;
> + } else {
> + err = -EOPNOTSUPP;
> + goto get_put_fail;
> + }
>
> if (param_item->driverinit_value_valid) {
> default_value[i] = param_item->driverinit_default;
> default_value_set[i] = true;
> }
> } else {
> - ctx.cmode = i;
> - err = devlink_param_get(devlink, param, &ctx, extack);
> + ctx->cmode = i;
> + err = devlink_param_get(devlink, param, ctx, extack);
> if (err)
> - return err;
> - param_value[i] = ctx.val;
> + goto get_put_fail;
> + param_value[i] = ctx->val;
>
> - err = devlink_param_get_default(devlink, param, &ctx,
> + err = devlink_param_get_default(devlink, param, ctx,
> extack);
> if (!err) {
> - default_value[i] = ctx.val;
> + default_value[i] = ctx->val;
> default_value_set[i] = true;
> } else if (err != -EOPNOTSUPP) {
> - return err;
> + goto get_put_fail;
> }
> }
> param_value_set[i] = true;
> }
>
> + err = -EMSGSIZE;
> hdr = genlmsg_put(msg, portid, seq, &devlink_nl_family, flags, cmd);
> if (!hdr)
> - return -EMSGSIZE;
> + goto get_put_fail;
>
> if (devlink_nl_put_handle(msg, devlink))
> goto genlmsg_cancel;
> @@ -393,6 +423,9 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
> nla_nest_end(msg, param_values_list);
> nla_nest_end(msg, param_attr);
> genlmsg_end(msg, hdr);
> + kfree(default_value);
> + kfree(param_value);
> + kfree(ctx);
> return 0;
>
> values_list_nest_cancel:
> @@ -401,7 +434,11 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
> nla_nest_cancel(msg, param_attr);
> genlmsg_cancel:
> genlmsg_cancel(msg, hdr);
> - return -EMSGSIZE;
> +get_put_fail:
> + kfree(default_value);
> + kfree(param_value);
> + kfree(ctx);
> + return err;
> }
>
> static void devlink_param_notify(struct devlink *devlink,
> @@ -507,7 +544,7 @@ devlink_param_value_get_from_info(const struct devlink_param *param,
> union devlink_param_value *value)
> {
> struct nlattr *param_data;
> - int len;
> + int len, cnt, rem;
>
> param_data = info->attrs[DEVLINK_ATTR_PARAM_VALUE_DATA];
>
> @@ -547,6 +584,26 @@ devlink_param_value_get_from_info(const struct devlink_param *param,
> return -EINVAL;
> value->vbool = nla_get_flag(param_data);
> break;
> +
> + case DEVLINK_PARAM_TYPE_U64_ARRAY:
> + cnt = 0;
> + nla_for_each_attr_type(param_data,
> + DEVLINK_ATTR_PARAM_VALUE_DATA,
> + genlmsg_data(info->genlhdr),
> + genlmsg_len(info->genlhdr), rem) {
> + if (cnt >= __DEVLINK_PARAM_MAX_ARRAY_SIZE)
> + return -EMSGSIZE;
> +
> + if ((nla_len(param_data) != sizeof(u64)) &&
> + (nla_len(param_data) != sizeof(u32)))
> + return -EINVAL;
> +
> + value->u64arr.val[cnt] = (u64)nla_get_uint(param_data);
> + cnt++;
> + }
> +
> + value->u64arr.size = cnt;
> + break;
Sashiko says:
---
Does this make it impossible to set an empty array to clear a
multi-value parameter?
If userspace provides 0 elements, param_data will be NULL. Earlier in
devlink_param_value_get_from_info(), there is a check:
param_data = info->attrs[DEVLINK_ATTR_PARAM_VALUE_DATA];
if (param->type != DEVLINK_PARAM_TYPE_BOOL && !param_data)
return -EINVAL;
If the parameter is a U64_ARRAY and no data is provided, this check will
immediately return -EINVAL.
The kernel can successfully emit an empty array on a GET request if the
size is 0. Should the SET path similarly support receiving 0 elements to
allow userspace to clear a multi-value parameter?
---
There are several others NIC-specific remarks, which IMHO are mostly
pre-existing issues, but please have a look:
https://sashiko.dev/#/patchset/20260403025533.6250-1-rkannoth%40marvell.com
/P
^ permalink raw reply
* Re: [PATCH net-next v11 00/14] netkit: Support for io_uring zero-copy and AF_XDP
From: Daniel Borkmann @ 2026-04-07 9:50 UTC (permalink / raw)
To: netdev
Cc: bpf, kuba, davem, razor, pabeni, willemb, sdf, john.fastabend,
martin.lau, jordan, maciej.fijalkowski, magnus.karlsson, dw, toke,
yangzhenze, wangdongdong.6
In-Reply-To: <20260402231031.447597-1-daniel@iogearbox.net>
Hi Jakub,
On 4/3/26 1:10 AM, Daniel Borkmann wrote:
> Containers use virtual netdevs to route traffic from a physical netdev
> in the host namespace. They do not have access to the physical netdev
> in the host and thus can't use memory providers or AF_XDP that require
> reconfiguring/restarting queues in the physical netdev.
>
> This patchset adds the concept of queue leasing to virtual netdevs that
> allow containers to use memory providers and AF_XDP at native speed.
> Leased queues are bound to a real queue in a physical netdev and act
> as a proxy.
>
> Memory providers and AF_XDP operations take an ifindex and queue id,
> so containers would pass in an ifindex for a virtual netdev and a queue
> id of a leased queue, which then gets proxied to the underlying real
> queue.
>
> We have implemented support for this concept in netkit and tested the
> latter against Nvidia ConnectX-6 (mlx5) as well as Broadcom BCM957504
> (bnxt_en) 100G NICs. For more details see the individual patches.
>
> v10->v11:
> - Fix missing mp_ops->uninstall upon unlease path (Gemini)
> - Fix dma device retrieval on tx queue when rx queue is leased (Gemini)
> - Rework ethtool channel checks to check rx/tx individually (Gemini)
> - The remainder of the Gemini findings were non-issues
> - Add more extensive net selftests around queue leasing corner cases
> - Rebase and retested everything with mlx5 + bnxt_en
I've look into the latest v11 findings from Gemini in the meantime, and
thought I'd share an update/comments related to them:
https://sashiko.dev/#/patchset/20260402231031.447597-1-daniel%40iogearbox.net
From all the claims related to the series, the one in the netkit device dump
looks reasonable to improve, iow, to not dump IFLA_NETKIT_SCRUB if in single
mode since we also reject the presence of the attribute during device creation,
change would look like:
- if (nla_put_u32(skb, IFLA_NETKIT_SCRUB, nk->scrub))
- return -EMSGSIZE;
+ if (nk->pair != NETKIT_DEVICE_SINGLE &&
+ nla_put_u32(skb, IFLA_NETKIT_SCRUB, nk->scrub))
+ return -EMSGSIZE;
Anyway, wrt the individual patch comments from Gemini:
Patch 2:
- The capable(CAP_NET_ADMIN) is intentional
Patch 6:
- Solved in your patch which is a good thing to do
https://patchwork.kernel.org/project/netdevbpf/patch/20260404001938.2425670-1-kuba@kernel.org/
-> Iiuc, its unreachable today due to other guards, but an explicit check
would definitely cement that
- The second double free or use-after-free on unregistered devices claim is
not true, there is an inconsistency in the io_uring vs devmem wrt clearing
mp_ops to NULL but the code handles both cases correctly
Patch 7:
- Independent of the series; oob queue index in io_uring/zcrx claims that
there is no bounds-check against real_num_rx_queues before being passed
to netdev_queue_get_dma_dev and from there into ndo_queue_get_dma_dev
callback
-> mlx5e_queue_get_dma_dev as the only implementation does bounds check
internally, it might make sense to have a guard in the core code if
more drivers implement the ndo_queue_get_dma_dev
- Order-dependent DMA dev validation in netdev_nl_get_dma_dev claim cannot
override memory provider state, mixed cases get rejected
Patch 9:
- Intentional, netkit proxies all xsk operations to the phys device
Patch 10:
- Above diff to not dump IFLA_NETKIT_SCRUB makes sense to me
- The changelink operations claim further below is intentional and for now
rejected with -EOPNOTSUPP given nothing new can be configured here
Patch 11:
- netkit notifier takes care of this and is part of this series
Some cases in patch 6 and 7 that don't require a specific driver are worth additional
tracking in the netkit leasing selftests to asset behavior though. Happy to fold that
+ the IFLA_NETKIT_SCRUB diff above into a v12 or as a follow-up, let me know.
Thanks a lot,
Daniel
^ permalink raw reply
* Re: [PATCH net-next] selftests: drv-net: adjust to socat changes
From: Breno Leitao @ 2026-04-07 9:43 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, shuah,
hawk, john.fastabend, sdf, linux-kselftest
In-Reply-To: <20260404230103.2719103-1-kuba@kernel.org>
On Sat, Apr 04, 2026 at 04:01:03PM -0700, Jakub Kicinski wrote:
> socat v1.8.1.0 now defaults to shut-null, it sends an extra
> 0-length UDP packet when sender disconnects. This breaks
> our tests which expect the exact packet sequence.
>
> Add shut-none which was the old default where necessary.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Breno Leitao <leitao@debian.org>
^ permalink raw reply
* [PATCH net v2] net: txgbe: fix RTNL assertion warning when remove module
From: Jiawen Wu @ 2026-04-07 9:40 UTC (permalink / raw)
To: netdev
Cc: Russell King, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Mengyuan Lou,
Jiawen Wu, stable
For the copper NIC with external PHY, the driver called
phylink_connect_phy() during probe and phylink_disconnect_phy() during
remove. It caused an RTNL assertion warning in phylink_disconnect_phy()
upon module remove.
To fix this, add rtnl_lock() and rtnl_unlock() around the
phylink_disconnect_phy() in remove function.
------------[ cut here ]------------
RTNL: assertion failed at drivers/net/phy/phylink.c (2351)
WARNING: drivers/net/phy/phylink.c:2351 at
phylink_disconnect_phy+0xd8/0xf0 [phylink], CPU#0: rmmod/4464
Modules linked in: ...
CPU: 0 UID: 0 PID: 4464 Comm: rmmod Kdump: loaded Not tainted 7.0.0-rc4+
Hardware name: Micro-Star International Co., Ltd. MS-7E16/X670E GAMING
PLUS WIFI (MS-7E16), BIOS 1.90 12/31/2024
RIP: 0010:phylink_disconnect_phy+0xe4/0xf0 [phylink]
Code: 5b 41 5c 41 5d 41 5e 41 5f 5d 31 c0 31 d2 31 f6 31 ff e9 3a 38 8f e7
48 8d 3d 48 87 e2 ff ba 2f 09 00 00 48 c7 c6 c1 22 24 c0 <67> 48 0f b9 3a
e9 34 ff ff ff 66 90 90 90 90 90 90 90 90 90 90 90
RSP: 0018:ffffce7288363ac0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff89654b2a1a00 RCX: 0000000000000000
RDX: 000000000000092f RSI: ffffffffc02422c1 RDI: ffffffffc0239020
RBP: ffffce7288363ae8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8964c4022000
R13: ffff89654fce3028 R14: ffff89654ebb4000 R15: ffffffffc0226348
FS: 0000795e80d93780(0000) GS:ffff896c52857000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005b528b592000 CR3: 0000000170d0f000 CR4: 0000000000f50ef0
PKRU: 55555554
Call Trace:
<TASK>
txgbe_remove_phy+0xbb/0xd0 [txgbe]
txgbe_remove+0x4c/0xb0 [txgbe]
pci_device_remove+0x41/0xb0
device_remove+0x43/0x80
device_release_driver_internal+0x206/0x270
driver_detach+0x4a/0xa0
bus_remove_driver+0x83/0x120
driver_unregister+0x2f/0x60
pci_unregister_driver+0x40/0x90
txgbe_driver_exit+0x10/0x850 [txgbe]
__do_sys_delete_module.isra.0+0x1c3/0x2f0
__x64_sys_delete_module+0x12/0x20
x64_sys_call+0x20c3/0x2390
do_syscall_64+0x11c/0x1500
? srso_alias_return_thunk+0x5/0xfbef5
? do_syscall_64+0x15a/0x1500
? srso_alias_return_thunk+0x5/0xfbef5
? do_fault+0x312/0x580
? srso_alias_return_thunk+0x5/0xfbef5
? __handle_mm_fault+0x9d5/0x1040
? srso_alias_return_thunk+0x5/0xfbef5
? count_memcg_events+0x101/0x1d0
? srso_alias_return_thunk+0x5/0xfbef5
? handle_mm_fault+0x1e8/0x2f0
? srso_alias_return_thunk+0x5/0xfbef5
? do_user_addr_fault+0x2f8/0x820
? srso_alias_return_thunk+0x5/0xfbef5
? irqentry_exit+0xb2/0x600
? srso_alias_return_thunk+0x5/0xfbef5
? exc_page_fault+0x92/0x1c0
entry_SYSCALL_64_after_hwframe+0x76/0x7e
Fixes: 02b2a6f91b90 ("net: txgbe: support copper NIC with external PHY")
Cc: stable@vger.kernel.org
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
v2:
- Add RTNL lock instead of moving phylink connect/disconnect functions.
v1: https://lore.kernel.org/all/D56A7C3379B4DA62+20260331071107.5414-1-jiawenwu@trustnetic.com
---
drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 8ea7aa07ae4e..dc9f24314658 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -657,7 +657,9 @@ void txgbe_remove_phy(struct txgbe *txgbe)
return;
case wx_mac_sp:
if (txgbe->wx->media_type == wx_media_copper) {
+ rtnl_lock();
phylink_disconnect_phy(txgbe->wx->phylink);
+ rtnl_unlock();
phylink_destroy(txgbe->wx->phylink);
return;
}
--
2.48.1
^ permalink raw reply related
* [PATCH net-next] ppp: consolidate refcount decrements
From: Qingfang Deng @ 2026-04-07 9:40 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Qingfang Deng, Sebastian Andrzej Siewior, Kees Cook,
Kuniyuki Iwashima, linux-ppp, netdev, linux-kernel
ppp_destroy_{channel,interface} are always called after
refcount_dec_and_test().
To reduce boilerplate code, consolidate the decrements by moving them
into the two functions. To reflect this change in semantics, rename the
functions to ppp_release_*.
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
drivers/net/ppp/ppp_generic.c | 61 ++++++++++++++++-------------------
1 file changed, 28 insertions(+), 33 deletions(-)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index cb29a6968c63..b097d1b38ac9 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -286,12 +286,12 @@ static struct compressor *find_compressor(int type);
static void ppp_get_stats(struct ppp *ppp, struct ppp_stats *st);
static int ppp_create_interface(struct net *net, struct file *file, int *unit);
static void init_ppp_file(struct ppp_file *pf, int kind);
-static void ppp_destroy_interface(struct ppp *ppp);
+static void ppp_release_interface(struct ppp *ppp);
static struct ppp *ppp_find_unit(struct ppp_net *pn, int unit);
static struct channel *ppp_find_channel(struct ppp_net *pn, int unit);
static int ppp_connect_channel(struct channel *pch, int unit);
static int ppp_disconnect_channel(struct channel *pch);
-static void ppp_destroy_channel(struct channel *pch);
+static void ppp_release_channel(struct channel *pch);
static int unit_get(struct idr *p, void *ptr, int min);
static int unit_set(struct idr *p, void *ptr, int n);
static void unit_put(struct idr *p, int n);
@@ -407,22 +407,18 @@ static int ppp_release(struct inode *unused, struct file *file)
if (pf) {
file->private_data = NULL;
- if (pf->kind == INTERFACE) {
+ switch (pf->kind) {
+ case INTERFACE:
ppp = PF_TO_PPP(pf);
rtnl_lock();
if (file == ppp->owner)
unregister_netdevice(ppp->dev);
rtnl_unlock();
- }
- if (refcount_dec_and_test(&pf->refcnt)) {
- switch (pf->kind) {
- case INTERFACE:
- ppp_destroy_interface(PF_TO_PPP(pf));
- break;
- case CHANNEL:
- ppp_destroy_channel(PF_TO_CHANNEL(pf));
- break;
- }
+ ppp_release_interface(ppp);
+ break;
+ case CHANNEL:
+ ppp_release_channel(PF_TO_CHANNEL(pf));
+ break;
}
}
return 0;
@@ -675,8 +671,7 @@ static int ppp_bridge_channels(struct channel *pch, struct channel *pchb)
synchronize_rcu();
if (pchb)
- if (refcount_dec_and_test(&pchb->file.refcnt))
- ppp_destroy_channel(pchb);
+ ppp_release_channel(pchb);
return -EALREADY;
}
@@ -708,11 +703,9 @@ static int ppp_unbridge_channels(struct channel *pch)
synchronize_rcu();
if (pchbb == pch)
- if (refcount_dec_and_test(&pch->file.refcnt))
- ppp_destroy_channel(pch);
+ ppp_release_channel(pch);
- if (refcount_dec_and_test(&pchb->file.refcnt))
- ppp_destroy_channel(pchb);
+ ppp_release_channel(pchb);
return 0;
}
@@ -786,8 +779,7 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
err = ppp_bridge_channels(pch, pchb);
/* Drop earlier refcount now bridge establishment is complete */
- if (refcount_dec_and_test(&pchb->file.refcnt))
- ppp_destroy_channel(pchb);
+ ppp_release_channel(pchb);
break;
case PPPIOCUNBRIDGECHAN:
@@ -1584,8 +1576,7 @@ static void ppp_dev_priv_destructor(struct net_device *dev)
struct ppp *ppp;
ppp = netdev_priv(dev);
- if (refcount_dec_and_test(&ppp->file.refcnt))
- ppp_destroy_interface(ppp);
+ ppp_release_interface(ppp);
}
static int ppp_fill_forward_path(struct net_device_path_ctx *ctx,
@@ -3022,8 +3013,7 @@ ppp_unregister_channel(struct ppp_channel *chan)
pch->file.dead = 1;
wake_up_interruptible(&pch->file.rwait);
- if (refcount_dec_and_test(&pch->file.refcnt))
- ppp_destroy_channel(pch);
+ ppp_release_channel(pch);
}
/*
@@ -3404,12 +3394,14 @@ init_ppp_file(struct ppp_file *pf, int kind)
}
/*
- * Free the memory used by a ppp unit. This is only called once
- * there are no channels connected to the unit and no file structs
- * that reference the unit.
+ * Drop a reference to a ppp unit and free its memory if the refcount reaches
+ * zero.
*/
-static void ppp_destroy_interface(struct ppp *ppp)
+static void ppp_release_interface(struct ppp *ppp)
{
+ if (!refcount_dec_and_test(&ppp->file.refcnt))
+ return;
+
atomic_dec(&ppp_unit_count);
if (!ppp->file.dead || ppp->n_channels) {
@@ -3561,18 +3553,21 @@ ppp_disconnect_channel(struct channel *pch)
wake_up_interruptible(&ppp->file.rwait);
ppp_unlock(ppp);
synchronize_net();
- if (refcount_dec_and_test(&ppp->file.refcnt))
- ppp_destroy_interface(ppp);
+ ppp_release_interface(ppp);
err = 0;
}
return err;
}
/*
- * Free up the resources used by a ppp channel.
+ * Drop a reference to a ppp channel and free its memory if the refcount reaches
+ * zero.
*/
-static void ppp_destroy_channel(struct channel *pch)
+static void ppp_release_channel(struct channel *pch)
{
+ if (!refcount_dec_and_test(&pch->file.refcnt))
+ return;
+
put_net_track(pch->chan_net, &pch->ns_tracker);
pch->chan_net = NULL;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net] net: initialize sk_rx_queue_mapping in sk_clone()
From: Eric Dumazet @ 2026-04-07 9:41 UTC (permalink / raw)
To: Jiayuan Chen
Cc: netdev, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Jakub Kicinski, Simon Horman,
Soheil Hassas Yeganeh, linux-kernel
In-Reply-To: <20260407084219.95718-1-jiayuan.chen@linux.dev>
On Tue, Apr 7, 2026 at 1:43 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> sk_clone() initializes sk_tx_queue_mapping via sk_tx_queue_clear()
> but does not initialize sk_rx_queue_mapping. Since this field is in
> the sk_dontcopy region, it is neither copied from the parent socket
> by sock_copy() nor zeroed by sk_prot_alloc() (called without
> __GFP_ZERO from sk_clone).
>
> Commit 03cfda4fa6ea ("tcp: fix another uninit-value
> (sk_rx_queue_mapping)") attempted to fix this by introducing
> sk_mark_napi_id_set() with force_set=true in tcp_child_process().
> However, sk_mark_napi_id_set() -> sk_rx_queue_set() only writes
> when skb_rx_queue_recorded(skb) is true. If the 3-way handshake
> ACK arrives through a device that does not record rx_queue (e.g.
> loopback or veth), sk_rx_queue_mapping remains uninitialized.
>
> When a subsequent data packet arrives with a recorded rx_queue,
> sk_mark_napi_id() -> sk_rx_queue_update() reads the uninitialized
> field for comparison (force_set=false path), triggering KMSAN.
>
> This was reproduced by establishing a TCP connection over loopback
> (which does not call skb_record_rx_queue), then attaching a BPF TC
> program on lo ingress to set skb->queue_mapping on data packets:
Ok, it's a somewhat convoluted way, and no real harm but KMSAN :)
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next v2 3/3] net: mdio: treat PSE EPROBE_DEFER as non-fatal during PHY registration
From: Kory Maincent @ 2026-04-07 9:40 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Andrew Lunn, Carlo Szelinsky, Andrew Lunn, Heiner Kallweit,
Russell King, Jakub Kicinski, David S . Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, netdev, linux-kernel
In-Reply-To: <adO_Za-X1cMjCEEa@pengutronix.de>
On Mon, 6 Apr 2026 16:12:53 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Mon, Apr 06, 2026 at 02:22:16PM +0200, Andrew Lunn wrote:
> > > The core question, do we need PSE for PHY functionality?
> >
> > I don't think the PHY should require PSE in order to send/receive
> > frames. If the PSE is not supplying power, the link peer is probably
> > off, but that is not so different from the cable being unplugged.
> >
> > > We can make a step back and re-evaluate - what functionality and what
> > > order is actually required to find potentially better implementation.
> > >
> > > We have a lot of current flowing over wires, budget and port
> > > prioritization issues, things which may damage HW if done not correctly.
> > > With other word, if we do not have properly operational environment
> > > providing system specific policies, it is better to run safe
> > > configuration - all ports/regulators are off.
> > >
> > > This means:
> > > - PSE controller driver should be registered as early as possible,
> > > without caring about existence of PHYs, ports or network interfaces.
> > > And configure ports in to default safe operation - off. Accept we
> > > have some controller/firmware which would care about safety.
> >
> > Don't most PSE have I2C or SPI interfaces? So they have a different
> > life cycle to PHYs, ports or netdevs. Only PSEs which are embedded
> > within a PHY, on an MDIO bus, will have a closely linked life
> > cycle. But do such devices exist?
>
> i'm not aware of any.
Same here, but it's not impossible to have such device in the future.
> > As soon as the PSE probes with all the resources it needs, and can
> > impose a safe default setting. And that can be independent of PHY and
> > netdev. I _think_ we only need the netdev for configuration, since
> > ethtool addresses netdev's. There would only be issues with user
> > space listening to udev creation events, it knows the PSE exists, but
> > it has no way to access it until the netdev is created.
>
> Ack. netdev is for configuration as virtual representation of PSE PI. And it
> is easier to assign corresponding interface for the LLDP.
> We do not really care about the PHY; it just happens to represent the
> port at the farthest end described in the Device Tree.
>
> > > - as soon as we have all needed components, we can start provide
> > > controllable interfaces to serve external consumers.
> >
> > Yep.
> >
> > > If we decouple PSE and PHY registration (and we probably will need to do
> > > it some day), we would need to have own implementation of deferred
> > > probing in the PSE core. Event driven or by polling - which sounds not
> > > like very good idea. Pick your poison...
> >
> > I don't see why. Maybe i'm missing something. We have two cases:
> >
> > 1) PSE probes first. When the PHY looks up the PSE, it exists, and it
> > is passed a handle to the PSE.
> >
> > 2) PHY probes first. The PSE core returns EPROBE_DEFFER, and the PHY
> > will try again later.
> >
> > I don't think there is any chicken/egg problems.
>
> Ack, i guess it is optimization problem.
Yes, as I reply in another mail of the thread:
We should be able to get the PSE control either at PSE register time or at PHY
(or PHY port) probe time.
Maybe if the PHY can't find the PSE PI, we could save the phandle of the PSE PI
somewhere in the PHY structure. Then at PSE register time, look for each PHY to
resolve every unresolved phandle.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH net-next v2 3/3] net: mdio: treat PSE EPROBE_DEFER as non-fatal during PHY registration
From: Kory Maincent @ 2026-04-07 9:31 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Carlo Szelinsky, Andrew Lunn, Andrew Lunn, Heiner Kallweit,
Russell King, Jakub Kicinski, David S . Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, netdev, linux-kernel
In-Reply-To: <adNx9z-AYxwpL2yH@pengutronix.de>
On Mon, 6 Apr 2026 10:42:31 +0200
Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> Hi,
>
> On Fri, Apr 03, 2026 at 03:38:49PM +0200, Kory Maincent wrote:
> > On Fri, 3 Apr 2026 15:31:11 +0200
> > Carlo Szelinsky <github@szelinsky.de> wrote:
> >
> > > Hi Andrew,
> > >
> > > Thx for helping me!
> > > I thought of keeping the eager path so we still catch broken DT bindings
> > > early at boot instead of silently failing later on first ethtool access.
> > > But you're right, dropping it would simplify things quite a bit. Do you
> > > think that trade-off is worth it? I will follow your lead.
> >
> > On my side I thinks that's a good idea, and I don't see any issue with that
> > for now. Oleksij you introduced it here in the first place, is it ok for
> > you?
>
> If I see it correctly - this patch kills all notifications originated
> from the PSE core to the users space, until some one calls get/set path
> from user space. Means, kernel update may break UAPI behavior of
> existing devices.
Indeed you are right, I have missed this point.
> On other hand, I agree that PSE is not a strickt requirement for PHY
> functionality in most cases. At the early stage, as initial PSE support
> was introduced, PHYs was kind of representation of the port and related
> ethernet interface (needed for LLDP). Are there better methods to solve
> it now?
Now that Maxime has merged the phy port representation (mdi) it could be binding
to it. But we should not break the current uAPI of having it bind directly
to the PHY. Thing is, the issue will be still present as PHY ports are only
created and initialized at probe time.
We should be able to get the PSE control either at PSE register time or at PHY
(or PHY port) probe time.
I just see there is more discussion about it in the thread.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [net-next PATCH 10/10] net: dsa: tag_rtl8_4: set KEEP flag
From: Linus Walleij @ 2026-04-07 9:22 UTC (permalink / raw)
To: Luiz Angelo Daros de Luca
Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Alvin Šipraga,
Yury Norov, Rasmus Villemoes, Russell King, netdev, linux-kernel
In-Reply-To: <20260331-realtek_forward-v1-10-44fb63033b7e@gmail.com>
On Wed, Apr 1, 2026 at 1:01 AM Luiz Angelo Daros de Luca
<luizluca@gmail.com> wrote:
> KEEP=1 is needed because we should respect the format of the packet as
> the kernel sends it to us. Unless tx forward offloading is used, the
> kernel is giving us the packet exactly as it should leave the specified
> port on the wire. Until now this was not needed because the ports were
> always functioning in a standalone mode in a VLAN-unaware way, so the
> switch would not tag or untag frames anyway. But arguably it should have
> been KEEP=1 all along.
>
> Co-developed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
I think you could just send this and patch 1 separately so they get
merged first as they are completely self-contained and uncontroversial
and touch the same code.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [net-next PATCH 09/10] net: dsa: realtek: rtl8365mb: add bridge port flags
From: Linus Walleij @ 2026-04-07 9:20 UTC (permalink / raw)
To: Luiz Angelo Daros de Luca
Cc: Andrew Lunn, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Alvin Šipraga,
Yury Norov, Rasmus Villemoes, Russell King, netdev, linux-kernel
In-Reply-To: <20260331-realtek_forward-v1-9-44fb63033b7e@gmail.com>
Hi Luiz, Alvin,
On Wed, Apr 1, 2026 at 1:01 AM Luiz Angelo Daros de Luca
<luizluca@gmail.com> wrote:
> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>
> Add bridge port flags for:
> - BR_LEARNING
> - BR_FLOOD
> - BR_MCAST_FLOOD
> - BR_BCAST_FLOOD
>
> Co-developed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
I have been working on a patch for RTL8366RB for this as well:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/patch/?id=0d58a9d74f363b969c06a8ac8446e6eef5511795
> +static int rtl8365mb_port_set_ucast_flood(struct realtek_priv *priv, int port,
> + bool enable)
> +static int rtl8365mb_port_set_mcast_flood(struct realtek_priv *priv, int port,
> + bool enable)
> +static int rtl8365mb_port_set_bcast_flood(struct realtek_priv *priv, int port,
> + bool enable)
Those are small and just writing registers so I guess they can be kept local,
but I think they should be accessed indirectly through
struct realtek_ops .rtl8366_port_set_ucast_flood() etc.
I can just make a simple patch adding the same three callbacks for
RTL8366RB and then it's done for both chips.
> +static int rtl8365mb_port_pre_bridge_flags(struct dsa_switch *ds, int port,
> + struct switchdev_brport_flags flags,
> + struct netlink_ext_ack *extack)
> +static int rtl8365mb_port_bridge_flags(struct dsa_switch *ds, int port,
> + struct switchdev_brport_flags flags,
> + struct netlink_ext_ack *exack)
Move this one to rtl8366-core.c under the name rtl8366_port_[pre_]bridge_flags()
and use the abstract accessors to do the register writes.
> @@ -1691,6 +1797,8 @@ static int rtl8365mb_port_bridge_join(struct dsa_switch *ds, int port,
> int ret;
> int i;
>
> + dev_dbg(priv->dev, "bridge %d join port %d\n", port, bridge.num);
> +
> /* Add this port to the isolation group of every other port
> * offloading this bridge.
> */
> @@ -1730,6 +1838,8 @@ static void rtl8365mb_port_bridge_leave(struct dsa_switch *ds, int port,
> u32 mask = 0;
> int i;
>
> + dev_dbg(priv->dev, "bridge %d leave port %d\n", port, bridge.num);
> +
> /* Remove this port from the isolation group of every other
> * port offloading this bridge.
> */
These debug prints seem unrelated to this patch?
> @@ -2547,6 +2657,19 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
> if (ret)
> goto out_teardown_irq;
>
> + /* Enable all types of flooding */
> + ret = rtl8365mb_port_set_ucast_flood(priv, i, true);
> + if (ret)
> + goto out_teardown_irq;
> +
> + ret = rtl8365mb_port_set_mcast_flood(priv, i, true);
> + if (ret)
> + goto out_teardown_irq;
> +
> + ret = rtl8365mb_port_set_bcast_flood(priv, i, true);
> + if (ret)
> + goto out_teardown_irq;
Create rtl8366_setup_flood_control() in realtek-core.c and add these
three calls as a helper using the rtl8366_* generic calls there. It will be
done the same on all chips.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH net v2 3/3] vsock/test: add MSG_PEEK after partial recv test
From: Luigi Leonardi @ 2026-04-07 9:13 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Arseniy Krasnov
Cc: kvm, virtualization, netdev, linux-kernel, Luigi Leonardi
In-Reply-To: <20260407-fix_peek-v2-0-2e2581dc8b7c@redhat.com>
Add a test that verifies MSG_PEEK works correctly after a partial
recv().
This is to test a bug that was present in the
`virtio_transport_stream_do_peek()` when computing the number of bytes to
copy: After a partial read, the peek function didn't take into
consideration the number of bytes that were already read. So peeking the
whole buffer would cause a out-of-bounds read, that resulted in a -EFAULT.
This test does exactly this: do a partial recv on a buffer, then try to
peek the whole buffer content.
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
tools/testing/vsock/vsock_test.c | 43 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index bdb0754965df..d38a90a86f34 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -346,6 +346,44 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
return test_msg_peek_server(opts, false);
}
+static void test_stream_peek_after_recv_client(const struct test_opts *opts)
+{
+ unsigned char buf[MSG_PEEK_BUF_LEN];
+ int fd;
+
+ fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ memset(buf, 0, sizeof(buf));
+
+ send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
+
+ close(fd);
+}
+
+static void test_stream_peek_after_recv_server(const struct test_opts *opts)
+{
+ unsigned char buf[MSG_PEEK_BUF_LEN];
+ int fd;
+
+ fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
+ if (fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Partial recv to advance offset within the skb */
+ recv_buf(fd, buf, 1, 0, 1);
+
+ /* Ask more bytes than available */
+ recv_buf(fd, buf, sizeof(buf), MSG_PEEK, sizeof(buf) - 1);
+
+ close(fd);
+}
+
#define SOCK_BUF_SIZE (2 * 1024 * 1024)
#define SOCK_BUF_SIZE_SMALL (64 * 1024)
#define MAX_MSG_PAGES 4
@@ -2509,6 +2547,11 @@ static struct test_case test_cases[] = {
.run_client = test_stream_tx_credit_bounds_client,
.run_server = test_stream_tx_credit_bounds_server,
},
+ {
+ .name = "SOCK_STREAM MSG_PEEK after partial recv",
+ .run_client = test_stream_peek_after_recv_client,
+ .run_server = test_stream_peek_after_recv_server,
+ },
{},
};
--
2.53.0
^ permalink raw reply related
* [PATCH net v2 2/3] vsock/test: handle MSG_PEEK in `recv_buf`
From: Luigi Leonardi @ 2026-04-07 9:13 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Arseniy Krasnov
Cc: kvm, virtualization, netdev, linux-kernel, Luigi Leonardi
In-Reply-To: <20260407-fix_peek-v2-0-2e2581dc8b7c@redhat.com>
`recv_buf` does not handle the MSG_PEEK flag correctly: it keeps calling
`recv` until all requested bytes are available or an error occurs.
The problem is how it calculates the amount of bytes read: MSG_PEEK
doesn't consume any bytes, will re-read the same bytes from the buffer
head, so, summing the return value every time is wrong.
Moreover, MSG_PEEK doesn't consume the bytes in the buffer, so if the
requested amount is more than the bytes available, the loop will never
terminate, because `recv` will never return EOF. For this reason we need
to compare the amount of read bytes with the number of bytes expected.
Add a check, and if the MSG_PEEK flag is present, update the counter of
read bytes differently, and break if we read the expected amount.
This allows us to simplify the `test_stream_credit_update_test`, by
reusing `recv_buf`.
Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
tools/testing/vsock/util.c | 8 ++++++++
tools/testing/vsock/vsock_test.c | 13 +------------
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 9430ef5b8bc3..f12425ca99ed 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -399,6 +399,14 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret)
if (ret == 0 || (ret < 0 && errno != EINTR))
break;
+ if (flags & MSG_PEEK) {
+ if (ret == expected_ret) {
+ nread = ret;
+ break;
+ }
+ continue;
+ }
+
nread += ret;
} while (nread < len);
timeout_end();
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 5bd20ccd9335..bdb0754965df 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -1500,18 +1500,7 @@ static void test_stream_credit_update_test(const struct test_opts *opts,
}
/* Wait until there will be 128KB of data in rx queue. */
- while (1) {
- ssize_t res;
-
- res = recv(fd, buf, buf_size, MSG_PEEK);
- if (res == buf_size)
- break;
-
- if (res <= 0) {
- fprintf(stderr, "unexpected 'recv()' return: %zi\n", res);
- exit(EXIT_FAILURE);
- }
- }
+ recv_buf(fd, buf, buf_size, MSG_PEEK, buf_size);
/* There is 128KB of data in the socket's rx queue, dequeue first
* 64KB, credit update is sent if 'low_rx_bytes_test' == true.
--
2.53.0
^ permalink raw reply related
* [PATCH net v2 1/3] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy
From: Luigi Leonardi @ 2026-04-07 9:13 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Arseniy Krasnov
Cc: kvm, virtualization, netdev, linux-kernel, Luigi Leonardi
In-Reply-To: <20260407-fix_peek-v2-0-2e2581dc8b7c@redhat.com>
`virtio_transport_stream_do_peek()` does not account for the skb offset
when computing the number of bytes to copy.
This means that, after a partial recv() that advances the offset, a peek
requesting more bytes than are available in the sk_buff causes
`skb_copy_datagram_iter()` to go past the valid payload, resulting in
a -EFAULT.
The dequeue path already handles this correctly.
Apply the same logic to the peek path.
Fixes: 0df7cd3c13e4 ("vsock/virtio/vhost: read data from non-linear skb")
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
---
net/vmw_vsock/virtio_transport_common.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 8a9fb23c6e85..4b65bfe5d875 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -547,9 +547,8 @@ virtio_transport_stream_do_peek(struct vsock_sock *vsk,
skb_queue_walk(&vvs->rx_queue, skb) {
size_t bytes;
- bytes = len - total;
- if (bytes > skb->len)
- bytes = skb->len;
+ bytes = min_t(size_t, len - total,
+ skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset);
spin_unlock_bh(&vvs->rx_lock);
--
2.53.0
^ permalink raw reply related
* [PATCH net v2 0/3] vsock/virtio: fix MSG_PEEK calculation on bytes to copy
From: Luigi Leonardi @ 2026-04-07 9:13 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
Jason Wang, Xuan Zhuo, Eugenio Pérez, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Arseniy Krasnov
Cc: kvm, virtualization, netdev, linux-kernel, Luigi Leonardi
`virtio_transport_stream_do_peek`, when calculating the number of bytes to
copy, didn't consider the `offset`, caused by partial reads that happened
before.
This might cause out-of-bounds read that lead to an EFAULT.
More details in the commits.
Commit 1 introduces the fix
Commit 2 introduces some preliminary work for adding a test
Commit 3 introduces a test that checks for this bug to avoid future
regressions.
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
Changes in v2:
- Addressed reviewers comment
- Test now uses the recv_buf utils.
- Removed unnecessary barrier
- Checkpatch warnings.
- Added new commit that allows to use recv_buf with MSG_PEEK
- Picked up RoBs
- Link to v1: https://lore.kernel.org/r/20260402-fix_peek-v1-0-ad274fcef77b@redhat.com
---
Luigi Leonardi (3):
vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy
vsock/test: handle MSG_PEEK in `recv_buf`
vsock/test: add MSG_PEEK after partial recv test
net/vmw_vsock/virtio_transport_common.c | 5 ++-
tools/testing/vsock/util.c | 8 +++++
tools/testing/vsock/vsock_test.c | 56 ++++++++++++++++++++++++++-------
3 files changed, 54 insertions(+), 15 deletions(-)
---
base-commit: bfe62a454542cfad3379f6ef5680b125f41e20f4
change-id: 20260401-fix_peek-6837b83469e3
Best regards,
--
Luigi Leonardi <leonardi@redhat.com>
^ 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