* Re: [PATCH net 1/1] ovpn: fix race between deleting interface and adding new peer
From: Sabrina Dubroca @ 2026-04-23 16:37 UTC (permalink / raw)
To: Antonio Quartulli
Cc: Jakub Kicinski, netdev, Ralf Lici, Paolo Abeni, Andrew Lunn,
David S. Miller, Eric Dumazet, Hyunwoo Kim
In-Reply-To: <9221605c-ad31-44f9-b3b7-db2237f75eb7@openvpn.net>
2026-04-23, 15:43:31 +0200, Antonio Quartulli wrote:
>
>
> On 23/04/2026 14:16, Antonio Quartulli wrote:
> > On 23/04/2026 04:20, Jakub Kicinski wrote:
> > > On Wed, 22 Apr 2026 14:32:42 +0200 Antonio Quartulli wrote:
> > > > + /* Prevent adding new peers while destroying the ovpn interface.
> > > > + * Failing to do so would end up holding the device reference
> > > > + * endlessly hostage of the new peer object with no chance of
> > > > + * release..
> > > > + */
> > > > + if (ovpn->dev->reg_state >= NETREG_UNREGISTERING)
> > > > + return -ENODEV;
> > >
> > > AI review suggests wrapping reg_state read in READ_ONCE(), I think
> > > that's legit. Also nit: I think > or != REGISTERED would be more
> > > idiomatic than comparing >= UNREGSITERING ?
> >
> > Agreed on READ_ONCE. Will fix it.
> >
> > As for your second point, I am fine with "!= REGISTERED" as that's the
> > only state we should be accepting new peers in any case.
> >
> > >
> > > If you agree make sure Sashiko doesn't complain about anything else
> > > once it's public:
> > > https://sashiko.dev/#/patchset/20260422123242.530882-2-
> > > antonio@openvpn.net
> >
> > Is there any way to get earlier access to these reviews? (at least for
> > patches somehow related to me/ovpn)
> >
>
> Dang! sashiko reminded me that I should swap cancel_delayed_work_sync with
> disable_delayed_work_sync. Will fix that.
>
>
> As for the second remarks..It has convincing arguments, but I need some more
> time to think about it.
>
> Sabrina, do you have an opinion?
Sadly, it seems possible.
I wanted to add rcu_read_lock around everything ovpn_peer_add() does,
so that unregister_netdevice_many_notify() gets stuck on the
synchronize_net() call after setting UNREGISTERING, but
ovpn_peer_add_p2p needs to be able to sleep [1]. If we move the
ovpn->lock from ovpn_peer_add_* to ovpn_peer_add, so that we check
reg_state and insert the new peer without being preempted, we should
be ok. But maybe submit that to netdev as RFC (not directly a pull
request) so that sashiko tells me how wrong I am?
We could also wrap all of ovpn_peer_add (including the new reg_state
check) in netdev_lock, since we can't move to NETREG_UNREGISTERING
until netdev_lock is released (then I think we wouldn't need the
READ_ONCE(reg_state)). But I don't know if that's an acceptable use of
netdev_lock.
[1] maybe we should add a might_sleep() annotation directly in
unlock_ovpn (instead of/in addition to the one already in
ovpn_socket_release), I'm already getting a bit rusty on the locking :/
--
Sabrina
^ permalink raw reply
* Re: [PATCH iwl-net v2] idpf: do not perform flow ops when netdev is detached
From: Simon Horman @ 2026-04-23 16:33 UTC (permalink / raw)
To: boolli
Cc: 'Simon Horman', anthony.l.nguyen, przemyslaw.kitszel,
davem, kuba, edumazet, intel-wired-lan, netdev, linux-kernel,
decot, anjali.singhai, sridhar.samudrala, brianvv,
emil.s.tantilov, stable
In-Reply-To: <20260421051641.370436-1-boolli@google.com>
From: 'Simon Horman' <horms@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
idpf: do not perform flow ops when netdev is detached
This commit adds checks to prevent crashes when out-of-tree modules bypass
ethtool core locks and call flow operations on detached netdevs during a
hardware reset. It introduces netdev and netif_device_present checks in
idpf_get_rxnfc and idpf_set_rxnfc.
> Even though commit 2e281e1155fc ("idpf: detach and close netdevs while
> handling a reset") prevents ethtool -N/-n operations to operate on
> detached netdevs, we found that out-of-tree workflows like OpenOnload
> can bypass ethtool core locks and call idpf_set_rxnfc directly during
> an idpf HW reset.
Is it acceptable to add defensive checks upstream solely to accommodate
out-of-tree modules that bypass core kernel locking?
The core ethtool infrastructure manages locks like rtnl_lock for in-tree
callers to guarantee safety. Modifying the upstream driver to paper over
locking violations in out-of-tree modules might contradict upstream policies.
Simon says: If this is to address OOT module behaviour then I am concerned.
And I would suggest that the correct approach is to fix
the OOT kernel module.
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> index bb99d9e7c65d5..8368a7e6a7544 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> @@ -43,6 +43,9 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
> unsigned int cnt = 0;
> int err = 0;
>
> + if (!netdev || !netif_device_present(netdev))
> + return -ENODEV;
> +
Could this result in undefined behavior if netdev is NULL? Before this check,
np is initialized using netdev_priv(netdev), which does pointer arithmetic.
If netdev can genuinely be NULL from out-of-tree callers, should the
initialization of np be moved after the validation?
Simon says: I think the actual problem here is that netdev is never NULL.
And thus the !netdev check should not be added.
Likewise for idpf_set_rxnfc()
Also, is there a time-of-check to time-of-use race here?
The netif_device_present() check is performed outside the vport_ctrl_lock
critical section.
If a concurrent hardware reset detaches the device and clears the present bit
before acquiring the vport_ctrl_lock to set np->vport = NULL, this thread
might pass the present check, block on idpf_vport_ctrl_lock(netdev), and
then dereference the newly set NULL vport once the lock is acquired.
Should the device presence check be moved inside the vport_ctrl_lock
critical section to prevent the NULL pointer dereference?
Simon says: This seems to be a legitimate concern.
Which seems to further suggest that the problem
should be addressed by the caller.
> idpf_vport_ctrl_lock(netdev);
[ ... ]
> @@ -349,6 +352,9 @@ static int idpf_set_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd)
> {
> int ret = -EOPNOTSUPP;
>
> + if (!netdev || !netif_device_present(netdev))
> + return -ENODEV;
> +
> idpf_vport_ctrl_lock(netdev);
Would this be vulnerable to the same time-of-check to time-of-use race
mentioned above, since the present check is outside the lock?
^ permalink raw reply
* Re: [RFC PATCH] net: skb: on zero-copy formatted output to skb
From: Andrew Lunn @ 2026-04-23 16:46 UTC (permalink / raw)
To: Dmitry Antipov
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev, linux-bluetooth
In-Reply-To: <65872064c2ab5fbf47a743f2dcd72182ea8296a7.camel@yandex.ru>
> > So please add some kerneldoc describing what it actually does.
>
> OK, some more neutral name like 'skb_fmt(...)' may be better indeed.
Sure. Just document what it actually does, what the return value
means, etc.
> > Please also annotate the const char *fmt, so the compiler can do
> > format string checks, parameter counting etc.
>
> Surely '__printf(2, 3)' should be used here.
Yep.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH net 1/1] ipv4: icmp: validate reply type before using icmp_pointers
From: Simon Horman @ 2026-04-23 16:58 UTC (permalink / raw)
To: Ren Wei
Cc: netdev, davem, dsahern, edumazet, kuba, pabeni,
andreas.a.roeseler, yuantan098, yifanwucs, tomapufckgml, bird,
caoruide123
In-Reply-To: <efb2c33f544ece7727704608fc577525b415ae26.1776563662.git.caoruide123@gmail.com>
On Sun, Apr 19, 2026 at 06:19:18PM +0800, Ren Wei wrote:
> From: Ruide Cao <caoruide123@gmail.com>
>
> Extended echo replies use ICMP_EXT_ECHOREPLY as the outbound reply type.
> That value is outside the range covered by icmp_pointers[], which only
> describes the traditional ICMP types up to NR_ICMP_TYPES.
>
> Avoid consulting icmp_pointers[] for reply types outside that range and
> keep the existing behavior for normal ICMP replies unchanged.
>
> Fixes: d329ea5bd884 ("icmp: add response to RFC 8335 PROBE messages")
> Cc: stable@kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Signed-off-by: Ruide Cao <caoruide123@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
Reviewed-by: Simon Horman <horms@kernel.org>
Sashiko has generated a review of this patch. I do not believe the
problems flagged there need to be addressed as the array indexes used in
those cases have are always valid. However, I would appreciate it if you
could look over this.
^ permalink raw reply
* Re: [PATCH net 1/1] ipv4: icmp: validate reply type before using icmp_pointers
From: Simon Horman @ 2026-04-23 17:00 UTC (permalink / raw)
To: Ren Wei
Cc: netdev, davem, dsahern, edumazet, kuba, pabeni,
andreas.a.roeseler, yuantan098, yifanwucs, tomapufckgml, bird,
caoruide123
In-Reply-To: <20260423165859.GE900403@horms.kernel.org>
On Thu, Apr 23, 2026 at 05:58:59PM +0100, Simon Horman wrote:
> On Sun, Apr 19, 2026 at 06:19:18PM +0800, Ren Wei wrote:
> > From: Ruide Cao <caoruide123@gmail.com>
> >
> > Extended echo replies use ICMP_EXT_ECHOREPLY as the outbound reply type.
> > That value is outside the range covered by icmp_pointers[], which only
> > describes the traditional ICMP types up to NR_ICMP_TYPES.
> >
> > Avoid consulting icmp_pointers[] for reply types outside that range and
> > keep the existing behavior for normal ICMP replies unchanged.
> >
> > Fixes: d329ea5bd884 ("icmp: add response to RFC 8335 PROBE messages")
> > Cc: stable@kernel.org
> > Reported-by: Yuan Tan <yuantan098@gmail.com>
> > Reported-by: Yifan Wu <yifanwucs@gmail.com>
> > Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> > Reported-by: Xin Liu <bird@lzu.edu.cn>
> > Signed-off-by: Ruide Cao <caoruide123@gmail.com>
> > Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
>
> Reviewed-by: Simon Horman <horms@kernel.org>
>
> Sashiko has generated a review of this patch. I do not believe the
> problems flagged there need to be addressed as the array indexes used in
> those cases have are always valid. However, I would appreciate it if you
> could look over this.
Sorry, I meant that to be a reply to v2.
I will respond there too.
^ permalink raw reply
* Re: [PATCH v11 00/12] VMSCAPE optimization for BHI variant
From: Dave Hansen @ 2026-04-23 17:00 UTC (permalink / raw)
To: Pawan Gupta, x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin,
Josh Poimboeuf, David Kaplan, Sean Christopherson,
Borislav Petkov, Dave Hansen, Peter Zijlstra, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, KP Singh, Jiri Olsa,
David S. Miller, David Laight, Andy Lutomirski, Thomas Gleixner,
Ingo Molnar, David Ahern, Martin KaFai Lau, Eduard Zingerman,
Song Liu, Yonghong Song, John Fastabend, Stanislav Fomichev,
Hao Luo, Paolo Bonzini, Jonathan Corbet, Jason Baron, Alice Ryhl,
Steven Rostedt, Ard Biesheuvel, Shuah Khan
Cc: linux-kernel, kvm, Asit Mallick, Tao Zhang, bpf, netdev,
linux-doc
In-Reply-To: <20260422-vmscape-bhb-v11-0-b18e0cf32af4@linux.intel.com>
On 4/22/26 23:14, Pawan Gupta wrote:
> v11:
> - Use ifdef EXPORT_SYMBOL_FOR_KVM guard for EXPORT_STATIC_CALL_FOR_KVM()
> definition. It is not practical to define one but not the other. (Sean)
> - Collected tags.
The feedback here seems to be winding down. I'll plan to queue this
after -rc1 unless something else big comes up.
^ permalink raw reply
* Re: [PATCH net v2] ipv4: icmp: validate reply type before using icmp_pointers
From: Simon Horman @ 2026-04-23 17:00 UTC (permalink / raw)
To: Ren Wei
Cc: netdev, edumazet, davem, dsahern, kuba, pabeni,
andreas.a.roeseler, yuantan098, yifanwucs, tomapufckgml, bird,
caoruide123
In-Reply-To: <0dace90c01a5978e829ca741ef684dbd7304ce62.1776628519.git.caoruide123@gmail.com>
On Tue, Apr 21, 2026 at 12:16:31PM +0800, Ren Wei wrote:
> From: Ruide Cao <caoruide123@gmail.com>
>
> Extended echo replies use ICMP_EXT_ECHOREPLY as the outbound reply type.
> That value is outside the range covered by icmp_pointers[], which only
> describes the traditional ICMP types up to NR_ICMP_TYPES.
>
> Avoid consulting icmp_pointers[] for reply types outside that range, and
> use array_index_nospec() for the remaining in-range lookup. Normal ICMP
> replies keep their existing behavior unchanged.
>
> Fixes: d329ea5bd884 ("icmp: add response to RFC 8335 PROBE messages")
> Cc: stable@kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Signed-off-by: Ruide Cao <caoruide123@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> changes in v2:
> use array_index_nospec()
> v1 link: https://lore.kernel.org/all/efb2c33f544ece7727704608fc577525b415ae26.1776563662.git.caoruide123@gmail.com/
Reviewed-by: Simon Horman <horms@kernel.org>
Sashiko has generated a review of this patch. I do not believe the
problems flagged there need to be addressed as the array indexes used in
those cases have are always valid. However, I would appreciate it if you
could look over this.
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH v2 iwl-net] i40e: keep q_vectors array in sync with channel count changes
From: Mekala, SunithaX D @ 2026-04-23 17:04 UTC (permalink / raw)
To: Fijalkowski, Maciej, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, Karlsson, Magnus, kuba@kernel.org,
pabeni@redhat.com, horms@kernel.org, Kitszel, Przemyslaw,
Keller, Jacob E, Fijalkowski, Maciej
In-Reply-To: <20260416114046.642171-1-maciej.fijalkowski@intel.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Maciej Fijalkowski
> Sent: Thursday, April 16, 2026 4:41 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Karlsson, Magnus <magnus.karlsson@intel.com>; kuba@kernel.org; pabeni@redhat.com; horms@kernel.org; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Keller, > Jacob E <jacob.e.keller@intel.com>; Fijalkowski, Maciej <maciej.fijalkowski@intel.com>
> Subject: [Intel-wired-lan] [PATCH v2 iwl-net] i40e: keep q_vectors array in sync with channel count changes
>
> For the main VSI, i40e_set_num_rings_in_vsi() always derives
> num_q_vectors from pf->num_lan_msix. At the same time, ethtool -L stores
> the user requested channel count in vsi->req_queue_pairs and the queue
> setup path uses that value for the effective number of queue pairs.
>
> This leaves queue and vector counts out of sync after shrinking channel
> count via ethtool -L. The active queue configuration is reduced, but the
> VSI still keeps the full PF-sized q_vector topology.
>
> That mismatch breaks reconfiguration flows which rely on vector/NAPI
> state matching the effective channel configuration. In particular,
> toggling /sys/class/net/<dev>/threaded after reducing the channel count
> can hang, and later channel-count changes can fail because VSI reinit
> does not rebuild q_vectors to match the new vector count.
>
> Fix this by making the main VSI num_q_vectors follow the effective
> requested channel count, capped by the available MSI-X vectors. Update
> i40e_vsi_reinit_setup() to rebuild q_vectors during VSI reinit so the
> vector topology is refreshed together with the ring arrays when channel
> count changes.
>
> Keep alloc_queue_pairs unchanged and based on pf->num_lan_qps so the VSI
> retains its full queue capacity.
>
> Selftest napi_threaded.py was originally used when Jakub reported hang
> on /sys/class/net/<dev>/threaded toggle. In order to make it pass on
> i40e, use persistent NAPI configuration for q_vector NAPIs so NAPI
> identity and threaded settings survive q_vector reallocation across
> channel-count changes. This is achieved by using netif_napi_add_config()
> when configuring q_vectors.
>
> $ export NETIF=ens259f1np1
> $ sudo -E env PATH="$PATH" ./tools/testing/selftests/drivers/net/napi_threaded.py
> TAP version 13
> 1..3
> ok 1 napi_threaded.napi_init
> ok 2 napi_threaded.change_num_queues
> ok 3 napi_threaded.enable_dev_threaded_disable_napi_threaded
> Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Reported-by: Jakub Kicinski <kuba@kernel.org>
> Closes: https://lore.kernel.org/intel-wired-lan/20260316133100.6054a11f@kernel.org/
> Fixes: d2a69fefd756 ("i40e: Fix changing previously set num_queue_pairs for PFs")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
> v2:
> - NULL vsi->tx_rings in i40e_vsi_alloc_arrays() (Sashiko)
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 35 +++++++++++++++++----
> 1 file changed, 29 insertions(+), 6 deletions(-)
Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
^ permalink raw reply
* Re: [PATCH net v3] ipv6: Cap TLV scan in ip6_tnl_parse_tlv_enc_lim
From: Justin Iurman @ 2026-04-23 17:07 UTC (permalink / raw)
To: Daniel Borkmann, kuba
Cc: edumazet, dsahern, tom, willemdebruijn.kernel, idosch, pabeni,
netdev
In-Reply-To: <20260421202406.717885-1-daniel@iogearbox.net>
On 4/21/26 22:24, Daniel Borkmann wrote:
> Commit 47d3d7ac656a ("ipv6: Implement limits on Hop-by-Hop and
> Destination options") added net.ipv6.max_{hbh,dst}_opts_{cnt,len}
> and applied them in ip6_parse_tlv(), the generic TLV walker
> invoked from ipv6_destopt_rcv() and ipv6_parse_hopopts().
>
> ip6_tnl_parse_tlv_enc_lim() does not go through ip6_parse_tlv();
> it has its own hand-rolled TLV scanner inside its NEXTHDR_DEST
> branch which looks for IPV6_TLV_TNL_ENCAP_LIMIT. That inner
> loop is bounded only by optlen, which can be up to 2048 bytes.
> Stuffing the Destination Options header with 2046 Pad1 (type=0)
> entries advances the scanner a single byte at a time, yielding
> ~2000 TLV iterations per extension header.
>
> Reusing max_dst_opts_cnt to bound the TLV iterations, matching
> the semantics from 47d3d7ac656a, would require duplicating
> ip6_parse_tlv() to also validate Pad1/PadN payload. It would
> also mandate enforcing max_dst_opts_len, since otherwise an
> attacker shifts the axis to few options with a giant PadN and
> recovers the original DoS. Allowing up to 8 options before the
> tunnel encapsulation limit TLV is liberal enough; in practice
> encap limit is the first TLV. Thus, go with a hard-coded limit
> IP6_TUNNEL_MAX_DEST_TLVS (8).
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Justin Iurman <justin.iurman@gmail.com>
Thanks, Daniel.
Cheers,
Justin
^ permalink raw reply
* Re: [PATCH v3] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Jakub Kicinski @ 2026-04-23 17:21 UTC (permalink / raw)
To: Lucien.Jheng
Cc: andrew, hkallweit1, linux, davem, edumazet, pabeni, netdev,
linux-kernel, bjorn, ericwouds, frank-w, daniel, lucien.jheng,
albert-al.lee
In-Reply-To: <20260420134506.35164-1-lucienzx159@gmail.com>
Since you need to repost to fix the fixes tag, plaese also improve the
error handling
On Mon, 20 Apr 2026 21:45:06 +0800 Lucien.Jheng wrote:
> static int an8811hb_probe(struct phy_device *phydev)
> {
> + struct mdio_device *mdiodev;
> struct en8811h_priv *priv;
> int ret;
>
> @@ -1175,10 +1281,22 @@ static int an8811hb_probe(struct phy_device *phydev)
> return -ENOMEM;
> phydev->priv = priv;
>
> + mdiodev = mdio_device_create(phydev->mdio.bus,
> + phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS);
> + if (IS_ERR(mdiodev))
> + return PTR_ERR(mdiodev);
> +
> + ret = mdio_device_register(mdiodev);
> + if (ret) {
goto err_dev_free;
> + mdio_device_free(mdiodev);
> + return ret;
> + }
> + priv->pbusdev = mdiodev;
> +
> ret = an8811hb_load_firmware(phydev);
> if (ret < 0) {
> phydev_err(phydev, "Load firmware failed: %d\n", ret);
> - return ret;
> + goto error;
goto err_dev_create;
> }
>
> en8811h_print_fw_version(phydev);
> @@ -1191,22 +1309,27 @@ static int an8811hb_probe(struct phy_device *phydev)
>
> ret = en8811h_leds_setup(phydev);
> if (ret < 0)
> - return ret;
> + goto error;
>
> priv->phydev = phydev;
> /* Co-Clock Output */
> ret = an8811hb_clk_provider_setup(&phydev->mdio.dev, &priv->hw);
> if (ret)
> - return ret;
> + goto error;
>
> /* Configure led gpio pins as output */
> ret = air_buckpbus_reg_modify(phydev, AN8811HB_GPIO_OUTPUT,
> AN8811HB_GPIO_OUTPUT_345,
> AN8811HB_GPIO_OUTPUT_345);
> if (ret < 0)
> - return ret;
> + goto error;
>
> return 0;
> +
> +error:
err_dev_free:
> + mdio_device_remove(priv->pbusdev);
err_dev_create:
> + mdio_device_free(priv->pbusdev);
> + return ret;
^ permalink raw reply
* Re: [PATCH v3] llc: Return -EINPROGRESS from llc_ui_connect()
From: Simon Horman @ 2026-04-23 17:26 UTC (permalink / raw)
To: Ernestas Kulik; +Cc: netdev, kuba, linux-kernel
In-Reply-To: <20260421060304.285419-1-ernestas.k@iconn-networks.com>
On Tue, Apr 21, 2026 at 09:02:26AM +0300, Ernestas Kulik wrote:
> Given a zero sk_sndtimeo, llc_ui_connect() skips waiting for state
> change and returns 0, confusing userspace applications that will assume
> the socket is connected, making e.g. getpeername() calls error out.
>
> More specifically, the issue was discovered in libcoap, where
> newly-added AF_LLC socket support was behaving differently from AF_INET
> connections due to EINPROGRESS handling being skipped.
>
> Set rc to -EINPROGRESS if connect() would not block, akin to AF_INET
> sockets.
>
> Signed-off-by: Ernestas Kulik <ernestas.k@iconn-networks.com>
> ---
> v2:
> - Add note about discovering the issue
> - Make rc assignment conditional
> v3:
> - Fix commit message after v2 changes
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply
* Re: [PATCH] net/sun: Fix multiple typos in comments
From: Jakub Kicinski @ 2026-04-23 17:27 UTC (permalink / raw)
To: Jakub Raczynski
Cc: netdev, linux-kernel, davem, andrew+netdev, kernel-janitors
In-Reply-To: <20260421114512.2690443-1-j.raczynski@samsung.com>
On Tue, 21 Apr 2026 13:45:12 +0200 Jakub Raczynski wrote:
> There are some typos in comments and while they are harmless and not visible,
> there is no reason not to fix them. Fix the ones that are not register related,
> which might have intentional naming convention.
## Form letter - net-next-closed
We have already submitted our pull request with net-next material for v7.1,
and therefore net-next is closed for new drivers, features, code refactoring
and optimizations. We are currently accepting bug fixes only.
Please repost when net-next reopens after Apr 27th.
RFC patches sent for review only are obviously welcome at any time.
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer
pv-bot: closed
^ permalink raw reply
* Re: [PATCH net 1/1] ovpn: fix race between deleting interface and adding new peer
From: Jakub Kicinski @ 2026-04-23 17:36 UTC (permalink / raw)
To: Sabrina Dubroca
Cc: Antonio Quartulli, netdev, Ralf Lici, Paolo Abeni, Andrew Lunn,
David S. Miller, Eric Dumazet, Hyunwoo Kim
In-Reply-To: <aepK3XXc8IFemb1Z@krikkit>
On Thu, 23 Apr 2026 18:37:49 +0200 Sabrina Dubroca wrote:
> We could also wrap all of ovpn_peer_add (including the new reg_state
> check) in netdev_lock, since we can't move to NETREG_UNREGISTERING
> until netdev_lock is released (then I think we wouldn't need the
> READ_ONCE(reg_state)). But I don't know if that's an acceptable use of
> netdev_lock.
Yup, fwiw it's a perfectly legitimate use case.
^ permalink raw reply
* Re: [PATCH net 1/1] ovpn: fix race between deleting interface and adding new peer
From: Jakub Kicinski @ 2026-04-23 17:38 UTC (permalink / raw)
To: Antonio Quartulli
Cc: netdev, Sabrina Dubroca, Ralf Lici, Paolo Abeni, Andrew Lunn,
David S. Miller, Eric Dumazet, Hyunwoo Kim
In-Reply-To: <7481bc31-44f6-43ae-b3aa-07002644d9e6@openvpn.net>
On Thu, 23 Apr 2026 14:16:52 +0200 Antonio Quartulli wrote:
> > If you agree make sure Sashiko doesn't complain about anything else
> > once it's public:
> > https://sashiko.dev/#/patchset/20260422123242.530882-2-antonio@openvpn.net
>
> Is there any way to get earlier access to these reviews? (at least for
> patches somehow related to me/ovpn)
netdev ones are blocked because noobs spam the list for a test.
Which you also appear to be trying to do :D So I shall count
that as a win, I guess? :)
The LLM is mostly just using the review prompts -
https://github.com/masoncl/review-prompts
point you favorite coding agent at that and ask it questions...
^ permalink raw reply
* Re: [PATCH 6.6.y] i40e: Fix preempt count leak in napi poll tracepoint
From: Jacob Keller @ 2026-04-23 17:38 UTC (permalink / raw)
To: charles_xu, tglx, anthony.l.nguyen, przemyslaw.kitszel,
intel-wired-lan, netdev, joe, aleksandr.loktionov, stable
In-Reply-To: <20260421071838.3878-1-charles_xu@189.cn>
On 4/21/2026 12:18 AM, charles_xu@189.cn wrote:
> From: Thomas Gleixner <tglx@kernel.org>
>
> [ Upstream commit 4b3d54a85bd37ebf2d9836f0d0de775c0ff21af9 ]
>
> Using get_cpu() in the tracepoint assignment causes an obvious preempt
> count leak because nothing invokes put_cpu() to undo it:
>
> softirq: huh, entered softirq 3 NET_RX with preempt_count 00000100, exited with 00000101?
>
> This clearly has seen a lot of testing in the last 3+ years...
>
> Use smp_processor_id() instead.
>
> Fixes: 6d4d584a7ea8 ("i40e: Add i40e_napi_poll tracepoint")
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org
> Reviewed-by: Joe Damato <joe@dama.to>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
> Signed-off-by: Charles Xu <charles_xu@189.cn>
> ---
This was already backported 6.12, but wasn't in v6.6. The v6.6 tree does
have the i40e_napi_poll tracepoint, so it makes sense to backport this
there as well.
Acked-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply
* Re: [PATCH net-next v5 00/10] enic: SR-IOV V2 admin channel and MBOX protocol
From: Jakub Kicinski @ 2026-04-23 17:39 UTC (permalink / raw)
To: Satish Kharat
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni, netdev,
linux-kernel, 20260401-enic-sriov-v2-prep-v4-0-d5834b2ef1b9,
Breno Leitao
In-Reply-To: <20260423-enic-sriov-v2-admin-channel-v2-v5-0-caa9f504a3dc@cisco.com>
On Thu, 23 Apr 2026 09:28:01 -0700 Satish Kharat wrote:
> This series adds the admin channel infrastructure and mailbox (MBOX)
> protocol needed for V2 SR-IOV support in the enic driver.
>
> The V2 SR-IOV design uses a direct PF-VF communication channel built on
> dedicated WQ/RQ/CQ hardware resources and an MSI-X interrupt.
## Form letter - net-next-closed
We have already submitted our pull request with net-next material for v7.1,
and therefore net-next is closed for new drivers, features, code refactoring
and optimizations. We are currently accepting bug fixes only.
Please repost when net-next reopens after Apr 27th.
RFC patches sent for review only are obviously welcome at any time.
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer
pv-bot: closed
^ permalink raw reply
* Re: [PATCH v4 net 2/3] net: mlx5e: fix CWR handling in drivers to preserve ACE signal
From: Paolo Abeni @ 2026-04-23 17:40 UTC (permalink / raw)
To: Dragos Tatulea, chia-yu.chang, linyunsheng, andrew+netdev, parav,
jasowang, mst, shenjian15, salil.mehta, shaojijie, saeedm, tariqt,
mbloch, leonro, linux-rdma, netdev, davem, edumazet, kuba, horms,
ij, ncardwell, koen.de_schepper, g.white, ingemar.s.johansson,
mirja.kuehlewind, cheshire, rs.ietf, Jason_Livingood, vidhi_goel
In-Reply-To: <e5d03a71-cfcd-417b-a3b3-94dbd6600f9d@nvidia.com>
On 4/23/26 4:19 PM, Dragos Tatulea wrote:
> On 23.04.26 09:30, Paolo Abeni wrote:
> [...]
>>> ---
>>> drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
>>> index 5b60aa47c75b..9b1c80079532 100644
>>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
>>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
>>> @@ -1180,7 +1180,7 @@ static void mlx5e_shampo_update_ipv4_tcp_hdr(struct mlx5e_rq *rq, struct iphdr *
>>> skb->csum_offset = offsetof(struct tcphdr, check);
>>>
>>> if (tcp->cwr)
>>> - skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
>>> + skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ACCECN;
>>
>> Here there is an open question for nVidia:
>>
> Sorry for missing this question in v3.
>
>> Is the above enough or will later segmentation lead to the wrong
>> results? I think/guess the firmware is (still) aggregating the wire
>> frames using the ECN schema, i.e. the first wire packet has CWR == 1,
>> the later CWR==0.
>>
> For mlx5 HW-GRO a packet with the CWR flag will flush the previous GRO session
> and will not start a GRO session for this packet (napi_gro_receive() will be
> called on this single segment skb).
>
> So this change won't impact the current GRO behavior from the mlx5 driver/hw side.
OK, thanks!
For my education: doesn't the above also means that mlx5 will never
build GSO packets with CWR set (and so the above statement should never
be reached)?
/P
^ permalink raw reply
* Re: [syzbot] [net?] general protection fault in kernel_sock_shutdown (4)
From: Jakub Kicinski @ 2026-04-23 17:41 UTC (permalink / raw)
To: syzbot
Cc: davem, dsahern, edumazet, horms, linux-kernel, netdev, pabeni,
syzkaller-bugs
In-Reply-To: <69ea344f.a00a0220.17a17.0040.GAE@google.com>
On Thu, 23 Apr 2026 08:01:35 -0700 syzbot wrote:
> Subject: [syzbot] [net?] general protection fault in kernel_sock_shutdown (4)
#syz set subsystems: rdma
^ permalink raw reply
* Re: [PATCH net v2 0/6] rxrpc: Miscellaneous fixes
From: Jakub Kicinski @ 2026-04-23 17:56 UTC (permalink / raw)
To: David Howells
Cc: netdev, Marc Dionne, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Anderson Nascimento, linux-afs, linux-kernel
In-Reply-To: <20260422161438.2593376-1-dhowells@redhat.com>
On Wed, 22 Apr 2026 17:14:29 +0100 David Howells wrote:
> Here are some fixes for rxrpc, as found by Sashiko[1]:
>
> (1) Fix leaks in rxkad_verify_response().
>
> (2) Fix handling of rxkad-encrypted packets with crypto-misaligned
> lengths.
>
> (3) Fix problem with unsharing DATA packets potentially causing a crash in
> the caller.
>
> (4) Fix lack of unsharing of RESPONSE packets.
>
> (5) Fix integer overflow in RxGK ticket length check.
>
> (6) Fix missing length check in RxKAD tickets.
Have you had a chance to look thru sashiko run for this?
https://sashiko.dev/#/patchset/20260422161438.2593376-4-dhowells@redhat.com
^ permalink raw reply
* Re: [PATCH net 1/1] net/smc: avoid early lgr access in smc_clc_wait_msg
From: patchwork-bot+netdevbpf @ 2026-04-23 18:10 UTC (permalink / raw)
To: Ren Wei
Cc: linux-rdma, linux-s390, netdev, alibuda, dust.li, sidraya, wenjia,
mjambigi, tonylu, guwen, davem, edumazet, kuba, pabeni, horms,
ubraun, yuantan098, yifanwucs, tomapufckgml, bird, ruijieli51
In-Reply-To: <08c68a5c817acf198cce63d22517e232e8d60718.1776850759.git.ruijieli51@gmail.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 22 Apr 2026 23:40:18 +0800 you wrote:
> From: Ruijie Li <ruijieli51@gmail.com>
>
> A CLC decline can be received while the handshake is still in an early
> stage, before the connection has been associated with a link group.
>
> The decline handling in smc_clc_wait_msg() updates link-group level sync
> state for first-contact declines, but that state only exists after link
> group setup has completed. Guard the link-group update accordingly and
> keep the per-socket peer diagnosis handling unchanged.
>
> [...]
Here is the summary with links:
- [net,1/1] net/smc: avoid early lgr access in smc_clc_wait_msg
https://git.kernel.org/netdev/net/c/5a8db80f721d
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] hv_sock: Return -EIO for malformed/short packets
From: patchwork-bot+netdevbpf @ 2026-04-23 18:10 UTC (permalink / raw)
To: Dexuan Cui
Cc: kys, haiyangz, wei.liu, longli, sgarzare, davem, edumazet, kuba,
pabeni, horms, niuxuewei.nxw, linux-hyperv, virtualization,
netdev, linux-kernel, stable
In-Reply-To: <20260423064811.1371749-1-decui@microsoft.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 22 Apr 2026 23:48:11 -0700 you wrote:
> Commit f63152958994 fixes a regression, however it fails to report an
> error for malformed/short packets -- normally we should never see such
> packets, but let's report an error for them just in case.
>
> Fixes: f63152958994 ("hv_sock: Report EOF instead of -EIO for FIN")
> Cc: stable@vger.kernel.org
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
>
> [...]
Here is the summary with links:
- [net,v2] hv_sock: Return -EIO for malformed/short packets
https://git.kernel.org/netdev/net/c/3d1f20727a63
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] virtio_net: sync rss_trailer.max_tx_vq on queue_pairs change via VQ_PAIRS_SET
From: patchwork-bot+netdevbpf @ 2026-04-23 18:10 UTC (permalink / raw)
To: Brett Creeley
Cc: mst, jasowang, andrew+netdev, davem, edumazet, kuba, pabeni,
netdev
In-Reply-To: <20260416212121.29073-1-brett.creeley@amd.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 16 Apr 2026 14:21:21 -0700 you wrote:
> When netif_is_rxfh_configured() is true (i.e., the user has explicitly
> configured the RSS indirection table), virtnet_set_queues() skips the
> RSS update path and falls through to the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET
> command to change the number of queue pairs. However, it does not update
> vi->rss_trailer.max_tx_vq to reflect the new queue_pairs value.
>
> This causes a mismatch between vi->curr_queue_pairs and
> vi->rss_trailer.max_tx_vq. Any subsequent RSS reconfiguration (e.g.,
> via ethtool -X) calls virtnet_commit_rss_command(), which sends the
> stale max_tx_vq to the device, silently reverting the queue count.
>
> [...]
Here is the summary with links:
- [net] virtio_net: sync rss_trailer.max_tx_vq on queue_pairs change via VQ_PAIRS_SET
https://git.kernel.org/netdev/net/c/3bc06da858ef
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] nfp: fix swapped arguments in nfp_encode_basic_qdr() calls
From: patchwork-bot+netdevbpf @ 2026-04-23 18:10 UTC (permalink / raw)
To: Alexey Kodanev
Cc: netdev, kuba, horms, andrew+netdev, davem, edumazet, pabeni,
oss-drivers
In-Reply-To: <20260422160536.61855-1-aleksei.kodanev@bell-sw.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 22 Apr 2026 16:05:36 +0000 you wrote:
> There is a mismatch between the passed arguments and the actual
> nfp_encode_basic_qdr() function parameter names:
>
> static int nfp_encode_basic_qdr(u64 addr, int dest_island, int cpp_tgt,
> int mode, bool addr40, int isld1,
> int isld0)
> {
> ...
>
> [...]
Here is the summary with links:
- [net,v2] nfp: fix swapped arguments in nfp_encode_basic_qdr() calls
https://git.kernel.org/netdev/net/c/4078c5611d75
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 bpf-next v3 4/9] bpf: Refactor object relationship tracking and fix dynptr UAF bug
From: Mykyta Yatsenko @ 2026-04-23 18:19 UTC (permalink / raw)
To: Amery Hung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, kernel-team
In-Reply-To: <20260421221016.2967924-5-ameryhung@gmail.com>
On 4/21/26 11:10 PM, Amery Hung wrote:
> Refactor object relationship tracking in the verifier and fix a dynptr
> use-after-free bug where file/skb dynptrs are not invalidated when the
> parent referenced object is freed.
>
> Add parent_id to bpf_reg_state to precisely track child-parent
> relationships. A child object's parent_id points to the parent object's
> id. This replaces the PTR_TO_MEM-specific dynptr_id and does not
> increase the size of bpf_reg_state on 64-bit machines as there is
> existing padding.
>
> When calling dynptr constructors (i.e., process_dynptr_func() with
> MEM_UNINIT argument), track the parent's id if the parent is a
> referenced object. This only applies to file dynptr and skb dynptr,
> so only pass parent reg->id to kfunc constructors.
>
> For release_reference(), invalidating an object now also invalidates
> all descendants by traversing the object tree. This is done using
> stack-based DFS to avoid recursive call chains of release_reference() ->
> unmark_stack_slots_dynptr() -> release_reference(). Referenced objects
> encountered during tree traversal cannot be indirectly released. They
> require an explicit helper/kfunc call to release the acquired resources.
>
> While the new design changes how object relationships are tracked in
> the verifier, it does not change the verifier's behavior. Here is the
> implication for dynptr, pointer casting, and owning/non-owning
> references:
>
> Dynptr:
>
> When initializing a dynptr, referenced dynptrs acquire a reference for
> ref_obj_id. If the dynptr has a referenced parent, parent_id tracks the
> parent's id. When cloning, ref_obj_id and parent_id are copied from the
> original. Releasing a referenced dynptr via release_reference(ref_obj_id)
> invalidates all clones and derived slices. For non-referenced dynptrs,
> only the specific dynptr and its children are invalidated.
>
> Pointer casting:
>
> Referenced socket pointers and their casted counterparts share the same
> lifetime but have different nullness — they have different id but the
> same ref_obj_id.
>
> Owning to non-owning reference conversion:
>
> After converting owning to non-owning by clearing ref_obj_id (e.g.,
> object(id=1, ref_obj_id=1) -> object(id=1, ref_obj_id=0)), the
> verifier only needs to release the reference state, so it calls
> release_reference_nomark() instead of release_reference().
>
> Note that the error message "reference has not been acquired before" in
> the helper and kfunc release paths is removed. This message was already
> unreachable. The verifier only calls release_reference() after
> confirming meta.ref_obj_id is valid, so the condition could never
> trigger in practice (no selftest exercises it either). With the
> refactor, release_reference() can now be called with non-acquired ids
> and have different error conditions. Report directly in
> release_reference() instead.
>
> Fixes: 870c28588afa ("bpf: net_sched: Add basic bpf qdisc kfuncs")
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> include/linux/bpf_verifier.h | 22 ++-
> kernel/bpf/log.c | 4 +-
> kernel/bpf/states.c | 9 +-
> kernel/bpf/verifier.c | 264 +++++++++++++++++------------------
> 4 files changed, 152 insertions(+), 147 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index dc0cff59246d..1314299c3763 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -65,7 +65,6 @@ struct bpf_reg_state {
>
> struct { /* for PTR_TO_MEM | PTR_TO_MEM_OR_NULL */
> u32 mem_size;
> - u32 dynptr_id; /* for dynptr slices */
> };
>
> /* For dynptr stack slots */
> @@ -193,6 +192,13 @@ struct bpf_reg_state {
> * allowed and has the same effect as bpf_sk_release(sk).
> */
> u32 ref_obj_id;
> + /* Tracks the parent object this register was derived from.
> + * Used for cascading invalidation: when the parent object is
> + * released or invalidated, all registers with matching parent_id
> + * are also invalidated. For example, a slice from bpf_dynptr_data()
> + * gets parent_id set to the dynptr's id.
> + */
> + u32 parent_id;
> /* Inside the callee two registers can be both PTR_TO_STACK like
> * R1=fp-8 and R2=fp-8, but one of them points to this function stack
> * while another to the caller's stack. To differentiate them 'frameno'
> @@ -508,7 +514,7 @@ struct bpf_verifier_state {
> iter < frame->allocated_stack / BPF_REG_SIZE; \
> iter++, reg = bpf_get_spilled_reg(iter, frame, mask))
>
> -#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr) \
> +#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __stack, __mask, __expr) \
> ({ \
> struct bpf_verifier_state *___vstate = __vst; \
> int ___i, ___j; \
> @@ -516,6 +522,7 @@ struct bpf_verifier_state {
> struct bpf_reg_state *___regs; \
> __state = ___vstate->frame[___i]; \
> ___regs = __state->regs; \
> + __stack = NULL; \
> for (___j = 0; ___j < MAX_BPF_REG; ___j++) { \
> __reg = &___regs[___j]; \
> (void)(__expr); \
> @@ -523,14 +530,19 @@ struct bpf_verifier_state {
> bpf_for_each_spilled_reg(___j, __state, __reg, __mask) { \
> if (!__reg) \
> continue; \
> + __stack = &__state->stack[___j]; \
> (void)(__expr); \
> } \
> } \
> })
>
> /* Invoke __expr over regsiters in __vst, setting __state and __reg */
> -#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
> - bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, 1 << STACK_SPILL, __expr)
> +#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
> + ({ \
> + struct bpf_stack_state * ___stack; \
> + bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, ___stack,\
> + 1 << STACK_SPILL, __expr); \
> + })
>
> /* linked list of verifier states used to prune search */
> struct bpf_verifier_state_list {
> @@ -1323,6 +1335,7 @@ struct bpf_dynptr_desc {
> enum bpf_dynptr_type type;
> u32 id;
> u32 ref_obj_id;
> + u32 parent_id;
> };
>
> struct bpf_kfunc_call_arg_meta {
> @@ -1334,6 +1347,7 @@ struct bpf_kfunc_call_arg_meta {
> const char *func_name;
> /* Out parameters */
> u32 ref_obj_id;
> + u32 id;
> u8 release_regno;
> bool r0_rdonly;
> u32 ret_btf_id;
> diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c
> index 011e4ec25acd..d8dd372e45cd 100644
> --- a/kernel/bpf/log.c
> +++ b/kernel/bpf/log.c
> @@ -667,6 +667,8 @@ static void print_reg_state(struct bpf_verifier_env *env,
> verbose(env, "%+d", reg->delta);
> if (reg->ref_obj_id)
> verbose_a("ref_obj_id=%d", reg->ref_obj_id);
> + if (reg->parent_id)
> + verbose_a("parent_id=%d", reg->parent_id);
> if (type_is_non_owning_ref(reg->type))
> verbose_a("%s", "non_own_ref");
> if (type_is_map_ptr(t)) {
> @@ -770,8 +772,6 @@ void print_verifier_state(struct bpf_verifier_env *env, const struct bpf_verifie
> verbose_a("id=%d", reg->id);
> if (reg->ref_obj_id)
> verbose_a("ref_id=%d", reg->ref_obj_id);
> - if (reg->dynptr_id)
> - verbose_a("dynptr_id=%d", reg->dynptr_id);
> verbose(env, ")");
> break;
> case STACK_ITER:
> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index 8478d2c6ed5b..72bd3bcda5fb 100644
> --- a/kernel/bpf/states.c
> +++ b/kernel/bpf/states.c
> @@ -494,7 +494,8 @@ static bool regs_exact(const struct bpf_reg_state *rold,
> {
> return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
> check_ids(rold->id, rcur->id, idmap) &&
> - check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
> + check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap) &&
> + check_ids(rold->parent_id, rcur->parent_id, idmap);
> }
>
> enum exact_level {
> @@ -619,7 +620,8 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
> range_within(rold, rcur) &&
> tnum_in(rold->var_off, rcur->var_off) &&
> check_ids(rold->id, rcur->id, idmap) &&
> - check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
> + check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap) &&
> + check_ids(rold->parent_id, rcur->parent_id, idmap);
> case PTR_TO_PACKET_META:
> case PTR_TO_PACKET:
> /* We must have at least as much range as the old ptr
> @@ -799,7 +801,8 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
> cur_reg = &cur->stack[spi].spilled_ptr;
> if (old_reg->dynptr.type != cur_reg->dynptr.type ||
> old_reg->dynptr.first_slot != cur_reg->dynptr.first_slot ||
> - !check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap))
> + !check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap) ||
> + !check_ids(old_reg->parent_id, cur_reg->parent_id, idmap))
> return false;
> break;
> case STACK_ITER:
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0313b7d5f6c9..908a3af0e7c4 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -201,7 +201,7 @@ struct bpf_verifier_stack_elem {
>
> static int acquire_reference(struct bpf_verifier_env *env, int insn_idx);
> static int release_reference_nomark(struct bpf_verifier_state *state, int ref_obj_id);
> -static int release_reference(struct bpf_verifier_env *env, int ref_obj_id);
> +static int release_reference(struct bpf_verifier_env *env, int id);
> static void invalidate_non_owning_refs(struct bpf_verifier_env *env);
> static bool in_rbtree_lock_required_cb(struct bpf_verifier_env *env);
> static int ref_set_non_owning(struct bpf_verifier_env *env,
> @@ -241,6 +241,7 @@ struct bpf_call_arg_meta {
> int mem_size;
> u64 msize_max_value;
> int ref_obj_id;
> + u32 id;
> int func_id;
> struct btf *btf;
> u32 btf_id;
> @@ -603,14 +604,14 @@ static enum bpf_type_flag get_dynptr_type_flag(enum bpf_dynptr_type type)
> }
> }
>
> -static bool dynptr_type_refcounted(enum bpf_dynptr_type type)
> +static bool dynptr_type_referenced(enum bpf_dynptr_type type)
> {
> return type == BPF_DYNPTR_TYPE_RINGBUF || type == BPF_DYNPTR_TYPE_FILE;
> }
>
> static void __mark_dynptr_reg(struct bpf_reg_state *reg,
> enum bpf_dynptr_type type,
> - bool first_slot, int dynptr_id);
> + bool first_slot, int id);
>
>
> static void mark_dynptr_stack_regs(struct bpf_verifier_env *env,
> @@ -635,11 +636,12 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env,
> struct bpf_func_state *state, int spi);
>
> static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
> - enum bpf_arg_type arg_type, int insn_idx, int clone_ref_obj_id)
> + enum bpf_arg_type arg_type, int insn_idx, int parent_id,
> + struct bpf_dynptr_desc *dynptr)
> {
> struct bpf_func_state *state = bpf_func(env, reg);
> + int spi, i, err, ref_obj_id = 0;
> enum bpf_dynptr_type type;
> - int spi, i, err;
>
> spi = dynptr_get_spi(env, reg);
> if (spi < 0)
> @@ -673,82 +675,56 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
> mark_dynptr_stack_regs(env, &state->stack[spi].spilled_ptr,
> &state->stack[spi - 1].spilled_ptr, type);
>
> - if (dynptr_type_refcounted(type)) {
> - /* The id is used to track proper releasing */
> - int id;
> -
> - if (clone_ref_obj_id)
> - id = clone_ref_obj_id;
> - else
> - id = acquire_reference(env, insn_idx);
> -
> - if (id < 0)
> - return id;
> -
> - state->stack[spi].spilled_ptr.ref_obj_id = id;
> - state->stack[spi - 1].spilled_ptr.ref_obj_id = id;
> + if (dynptr->type == BPF_DYNPTR_TYPE_INVALID) { /* dynptr constructors */
> + if (dynptr_type_referenced(type)) {
> + ref_obj_id = acquire_reference(env, insn_idx);
> + if (ref_obj_id < 0)
> + return ref_obj_id;
> + }
> + } else { /* bpf_dynptr_clone() */
> + ref_obj_id = dynptr->ref_obj_id;
> + parent_id = dynptr->parent_id;
> }
>
> + state->stack[spi].spilled_ptr.ref_obj_id = ref_obj_id;
> + state->stack[spi - 1].spilled_ptr.ref_obj_id = ref_obj_id;
> + state->stack[spi].spilled_ptr.parent_id = parent_id;
> + state->stack[spi - 1].spilled_ptr.parent_id = parent_id;
> +
> return 0;
> }
>
> -static void invalidate_dynptr(struct bpf_verifier_env *env, struct bpf_func_state *state, int spi)
> +static void invalidate_dynptr(struct bpf_verifier_env *env, struct bpf_func_state *state,
it looks like state is no longer needed.
> + struct bpf_stack_state *stack)
> {
> int i;
>
> for (i = 0; i < BPF_REG_SIZE; i++) {
> - state->stack[spi].slot_type[i] = STACK_INVALID;
> - state->stack[spi - 1].slot_type[i] = STACK_INVALID;
> + stack[0].slot_type[i] = STACK_INVALID;
> + stack[1].slot_type[i] = STACK_INVALID;
> }
>
> - bpf_mark_reg_not_init(env, &state->stack[spi].spilled_ptr);
> - bpf_mark_reg_not_init(env, &state->stack[spi - 1].spilled_ptr);
> + bpf_mark_reg_not_init(env, &stack[0].spilled_ptr);
> + bpf_mark_reg_not_init(env, &stack[1].spilled_ptr);
> }
>
> static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
> {
> struct bpf_func_state *state = bpf_func(env, reg);
> - int spi, ref_obj_id, i;
> + int spi;
>
> spi = dynptr_get_spi(env, reg);
> if (spi < 0)
> return spi;
>
> - if (!dynptr_type_refcounted(state->stack[spi].spilled_ptr.dynptr.type)) {
> - invalidate_dynptr(env, state, spi);
> - return 0;
> - }
> -
> - ref_obj_id = state->stack[spi].spilled_ptr.ref_obj_id;
> -
> - /* If the dynptr has a ref_obj_id, then we need to invalidate
> - * two things:
> - *
> - * 1) Any dynptrs with a matching ref_obj_id (clones)
> - * 2) Any slices derived from this dynptr.
> + /*
> + * For referenced dynptr, the clones share the same ref_obj_id and will be
> + * invalidated too. For non-referenced dynptr, only the dynptr and slices
> + * derived from it will be invalidated.
> */
> -
> - /* Invalidate any slices associated with this dynptr */
> - WARN_ON_ONCE(release_reference(env, ref_obj_id));
> -
> - /* Invalidate any dynptr clones */
> - for (i = 1; i < state->allocated_stack / BPF_REG_SIZE; i++) {
> - if (state->stack[i].spilled_ptr.ref_obj_id != ref_obj_id)
> - continue;
> -
> - /* it should always be the case that if the ref obj id
> - * matches then the stack slot also belongs to a
> - * dynptr
> - */
> - if (state->stack[i].slot_type[0] != STACK_DYNPTR) {
> - verifier_bug(env, "misconfigured ref_obj_id");
> - return -EFAULT;
> - }
> - if (state->stack[i].spilled_ptr.dynptr.first_slot)
> - invalidate_dynptr(env, state, i);
> - }
> -
> - return 0;
> + reg = &state->stack[spi].spilled_ptr;
> + return release_reference(env, dynptr_type_referenced(reg->dynptr.type) ?
> + reg->ref_obj_id : reg->id);
> }
>
> static void __mark_reg_unknown(const struct bpf_verifier_env *env,
> @@ -765,10 +741,6 @@ static void mark_reg_invalid(const struct bpf_verifier_env *env, struct bpf_reg_
> static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env,
> struct bpf_func_state *state, int spi)
> {
> - struct bpf_func_state *fstate;
> - struct bpf_reg_state *dreg;
> - int i, dynptr_id;
> -
> /* We always ensure that STACK_DYNPTR is never set partially,
> * hence just checking for slot_type[0] is enough. This is
> * different for STACK_SPILL, where it may be only set for
> @@ -781,9 +753,9 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env,
> if (!state->stack[spi].spilled_ptr.dynptr.first_slot)
> spi = spi + 1;
>
> - if (dynptr_type_refcounted(state->stack[spi].spilled_ptr.dynptr.type)) {
> + if (dynptr_type_referenced(state->stack[spi].spilled_ptr.dynptr.type)) {
> int ref_obj_id = state->stack[spi].spilled_ptr.ref_obj_id;
> - int ref_cnt = 0;
> + int i, ref_cnt = 0;
>
> /*
> * A referenced dynptr can be overwritten only if there is at
> @@ -808,29 +780,8 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env,
> mark_stack_slot_scratched(env, spi);
> mark_stack_slot_scratched(env, spi - 1);
>
> - /* Writing partially to one dynptr stack slot destroys both. */
> - for (i = 0; i < BPF_REG_SIZE; i++) {
> - state->stack[spi].slot_type[i] = STACK_INVALID;
> - state->stack[spi - 1].slot_type[i] = STACK_INVALID;
> - }
> -
> - dynptr_id = state->stack[spi].spilled_ptr.id;
> - /* Invalidate any slices associated with this dynptr */
> - bpf_for_each_reg_in_vstate(env->cur_state, fstate, dreg, ({
> - /* Dynptr slices are only PTR_TO_MEM_OR_NULL and PTR_TO_MEM */
> - if (dreg->type != (PTR_TO_MEM | PTR_MAYBE_NULL) && dreg->type != PTR_TO_MEM)
> - continue;
> - if (dreg->dynptr_id == dynptr_id)
> - mark_reg_invalid(env, dreg);
> - }));
> -
> - /* Do not release reference state, we are destroying dynptr on stack,
> - * not using some helper to release it. Just reset register.
> - */
> - bpf_mark_reg_not_init(env, &state->stack[spi].spilled_ptr);
> - bpf_mark_reg_not_init(env, &state->stack[spi - 1].spilled_ptr);
> -
> - return 0;
> + /* Invalidate the dynptr and any derived slices */
> + return release_reference(env, state->stack[spi].spilled_ptr.id);
> }
>
> static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
> @@ -1449,15 +1400,15 @@ static void release_reference_state(struct bpf_verifier_state *state, int idx)
> return;
> }
>
> -static bool find_reference_state(struct bpf_verifier_state *state, int ptr_id)
> +static struct bpf_reference_state *find_reference_state(struct bpf_verifier_state *state, int ptr_id)
> {
> int i;
>
> for (i = 0; i < state->acquired_refs; i++)
> if (state->refs[i].id == ptr_id)
> - return true;
> + return &state->refs[i];
>
> - return false;
> + return NULL;
> }
>
> static int release_lock_state(struct bpf_verifier_state *state, int type, int id, void *ptr)
> @@ -1764,6 +1715,7 @@ static void __mark_reg_known(struct bpf_reg_state *reg, u64 imm)
> offsetof(struct bpf_reg_state, var_off) - sizeof(reg->type));
> reg->id = 0;
> reg->ref_obj_id = 0;
> + reg->parent_id = 0;
> ___mark_reg_known(reg, imm);
> }
>
> @@ -1801,7 +1753,7 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env,
> }
>
> static void __mark_dynptr_reg(struct bpf_reg_state *reg, enum bpf_dynptr_type type,
> - bool first_slot, int dynptr_id)
> + bool first_slot, int id)
> {
> /* reg->type has no meaning for STACK_DYNPTR, but when we set reg for
> * callback arguments, it does need to be CONST_PTR_TO_DYNPTR, so simply
> @@ -1810,7 +1762,7 @@ static void __mark_dynptr_reg(struct bpf_reg_state *reg, enum bpf_dynptr_type ty
> __mark_reg_known_zero(reg);
> reg->type = CONST_PTR_TO_DYNPTR;
> /* Give each dynptr a unique id to uniquely associate slices to it. */
> - reg->id = dynptr_id;
> + reg->id = id;
> reg->dynptr.type = type;
> reg->dynptr.first_slot = first_slot;
> }
> @@ -2451,6 +2403,7 @@ void bpf_mark_reg_unknown_imprecise(struct bpf_reg_state *reg)
> reg->type = SCALAR_VALUE;
> reg->id = 0;
> reg->ref_obj_id = 0;
> + reg->parent_id = 0;
nit: it looks like we can do memset(0) on the entire reg and then set
fields that need to be non-zero: type, var_off + __mark_reg_unbounded().
> reg->var_off = tnum_unknown;
> reg->frameno = 0;
> reg->precise = false;
> @@ -7427,7 +7380,7 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno,
> * and checked dynamically during runtime.
> */
> static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn_idx,
> - enum bpf_arg_type arg_type, int clone_ref_obj_id,
> + enum bpf_arg_type arg_type, int parent_id,
> struct bpf_dynptr_desc *dynptr)
> {
> struct bpf_reg_state *reg = reg_state(env, regno);
> @@ -7470,7 +7423,8 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn
> return err;
> }
>
> - err = mark_stack_slots_dynptr(env, reg, arg_type, insn_idx, clone_ref_obj_id);
> + err = mark_stack_slots_dynptr(env, reg, arg_type, insn_idx, parent_id,
> + dynptr);
> } else /* OBJ_RELEASE and None case from above */ {
> /* For the reg->type == PTR_TO_STACK case, bpf_dynptr is never const */
> if (reg->type == CONST_PTR_TO_DYNPTR && (arg_type & OBJ_RELEASE)) {
> @@ -7507,6 +7461,7 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn
> dynptr->id = reg->id;
> dynptr->type = reg->dynptr.type;
> dynptr->ref_obj_id = reg->ref_obj_id;
> + dynptr->parent_id = reg->parent_id;
> }
> }
> return err;
> @@ -8461,7 +8416,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> */
> if (reg->type == PTR_TO_STACK) {
> spi = dynptr_get_spi(env, reg);
> - if (spi < 0 || !state->stack[spi].spilled_ptr.ref_obj_id) {
> + if (spi < 0 || !state->stack[spi].spilled_ptr.id) {
> verbose(env, "arg %d is an unacquired reference\n", regno);
> return -EINVAL;
> }
> @@ -8489,6 +8444,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> return -EACCES;
> }
> meta->ref_obj_id = reg->ref_obj_id;
> + meta->id = reg->id;
> }
>
> switch (base_type(arg_type)) {
> @@ -9111,26 +9067,75 @@ static int release_reference_nomark(struct bpf_verifier_state *state, int ref_ob
> return -EINVAL;
> }
>
> -/* The pointer with the specified id has released its reference to kernel
> - * resources. Identify all copies of the same pointer and clear the reference.
> - *
> - * This is the release function corresponding to acquire_reference(). Idempotent.
> - */
> -static int release_reference(struct bpf_verifier_env *env, int ref_obj_id)
> +static int idstack_push(struct bpf_idmap *idmap, u32 id)
> +{
> + int i;
> +
> + if (!id)
> + return 0;
> +
> + for (i = 0; i < idmap->cnt; i++)
> + if (idmap->map[i].old == id)
> + return 0;
> +
> + if (WARN_ON_ONCE(idmap->cnt >= BPF_ID_MAP_SIZE))
> + return -EFAULT;
It feels like this check belongs above, maybe the first thing to do.
> +
> + idmap->map[idmap->cnt++].old = id;
> + return 0;
> +}
> +
> +static int idstack_pop(struct bpf_idmap *idmap)
> {
> + if (!idmap->cnt)
> + return 0;
> +
> + return idmap->map[--idmap->cnt].old;
> +}
> +
> +/* Release id and objects referencing the id iteratively in a DFS manner */
> +static int release_reference(struct bpf_verifier_env *env, int id)
> +{
> + u32 mask = (1 << STACK_SPILL) | (1 << STACK_DYNPTR);
> struct bpf_verifier_state *vstate = env->cur_state;
> + struct bpf_idmap *idstack = &env->idmap_scratch;
> + struct bpf_stack_state *stack;
> struct bpf_func_state *state;
> struct bpf_reg_state *reg;
> - int err;
> + int root_id = id, err;
>
> - err = release_reference_nomark(vstate, ref_obj_id);
> - if (err)
> - return err;
> + idstack->cnt = 0;
> + idstack_push(idstack, id);
>
> - bpf_for_each_reg_in_vstate(vstate, state, reg, ({
> - if (reg->ref_obj_id == ref_obj_id)
> - mark_reg_invalid(env, reg);
> - }));
> + if (find_reference_state(vstate, id))
> + WARN_ON_ONCE(release_reference_nomark(vstate, id));
> +
> + while ((id = idstack_pop(idstack))) {
> + bpf_for_each_reg_in_vstate_mask(vstate, state, reg, stack, mask, ({
> + if (reg->id != id && reg->parent_id != id && reg->ref_obj_id != id)
> + continue;
> +
> + if (reg->ref_obj_id && id != root_id) {
Does this line check that the only ref_obj_id we should see is either
0 or root_id? can we rewrite it as
if (reg->ref_obj_id && reg->ref_obj_id != root_id)
this is simpler, because id can also be reg->id/reg->parent_id, which is
hard to reason what that means.
> + struct bpf_reference_state *ref_state;
> +
> + ref_state = find_reference_state(env->cur_state, reg->ref_obj_id);
> + verbose(env, "Unreleased reference id=%d alloc_insn=%d when releasing id=%d\n",
> + ref_state->id, ref_state->insn_idx, root_id);
> + return -EINVAL;
> + }
> +
> + if (reg->id != id) {
> + err = idstack_push(idstack, reg->id);
> + if (err)
> + return err;
> + }
> +
> + if (!stack || stack->slot_type[BPF_REG_SIZE - 1] == STACK_SPILL)
> + mark_reg_invalid(env, reg);
> + else if (stack->slot_type[BPF_REG_SIZE - 1] == STACK_DYNPTR)
> + invalidate_dynptr(env, state, stack);
> + }));
> + }
>
> return 0;
> }
> @@ -10298,11 +10303,8 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> */
> err = 0;
> }
> - if (err) {
> - verbose(env, "func %s#%d reference has not been acquired before\n",
> - func_id_name(func_id), func_id);
> + if (err)
> return err;
> - }
> }
>
> switch (func_id) {
> @@ -10580,10 +10582,8 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> regs[BPF_REG_0].ref_obj_id = id;
> }
>
> - if (func_id == BPF_FUNC_dynptr_data) {
> - regs[BPF_REG_0].dynptr_id = meta.dynptr.id;
> - regs[BPF_REG_0].ref_obj_id = meta.dynptr.ref_obj_id;
> - }
> + if (func_id == BPF_FUNC_dynptr_data)
> + regs[BPF_REG_0].parent_id = meta.dynptr.id;
>
> err = do_refine_retval_range(env, regs, fn->ret_type, func_id, &meta);
> if (err)
> @@ -12009,6 +12009,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> return -EFAULT;
> }
> meta->ref_obj_id = reg->ref_obj_id;
> + meta->id = reg->id;
> if (is_kfunc_release(meta))
> meta->release_regno = regno;
> }
> @@ -12145,7 +12146,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> case KF_ARG_PTR_TO_DYNPTR:
> {
> enum bpf_arg_type dynptr_arg_type = ARG_PTR_TO_DYNPTR;
> - int clone_ref_obj_id = 0;
>
> if (is_kfunc_arg_uninit(btf, &args[i]))
> dynptr_arg_type |= MEM_UNINIT;
> @@ -12171,15 +12171,10 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> }
>
> dynptr_arg_type |= (unsigned int)get_dynptr_type_flag(parent_type);
> - clone_ref_obj_id = meta->dynptr.ref_obj_id;
> - if (dynptr_type_refcounted(parent_type) && !clone_ref_obj_id) {
> - verifier_bug(env, "missing ref obj id for parent of clone");
> - return -EFAULT;
> - }
> }
>
> - ret = process_dynptr_func(env, regno, insn_idx, dynptr_arg_type, clone_ref_obj_id,
> - &meta->dynptr);
> + ret = process_dynptr_func(env, regno, insn_idx, dynptr_arg_type,
> + meta->ref_obj_id ? meta->id : 0, &meta->dynptr);
> if (ret < 0)
> return ret;
> break;
> @@ -12813,12 +12808,7 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_ca
> verifier_bug(env, "no dynptr id");
> return -EFAULT;
> }
> - regs[BPF_REG_0].dynptr_id = meta->dynptr.id;
> -
> - /* we don't need to set BPF_REG_0's ref obj id
> - * because packet slices are not refcounted (see
> - * dynptr_type_refcounted)
> - */
> + regs[BPF_REG_0].parent_id = meta->dynptr.id;
> } else {
> return 0;
> }
> @@ -12953,6 +12943,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> if (rcu_lock) {
> env->cur_state->active_rcu_locks++;
> } else if (rcu_unlock) {
> + struct bpf_stack_state *stack;
> struct bpf_func_state *state;
> struct bpf_reg_state *reg;
> u32 clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER);
> @@ -12962,7 +12953,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> return -EINVAL;
> }
> if (--env->cur_state->active_rcu_locks == 0) {
> - bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, clear_mask, ({
> + bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, stack, clear_mask, ({
> if (reg->type & MEM_RCU) {
> reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
> reg->type |= PTR_UNTRUSTED;
> @@ -13005,9 +12996,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> err = unmark_stack_slots_dynptr(env, reg);
> } else {
> err = release_reference(env, reg->ref_obj_id);
> - if (err)
> - verbose(env, "kfunc %s#%d reference has not been acquired before\n",
> - func_name, meta.func_id);
> }
> if (err)
> return err;
> @@ -13024,7 +13012,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> return err;
> }
>
> - err = release_reference(env, release_ref_obj_id);
> + err = release_reference_nomark(env->cur_state, release_ref_obj_id);
> if (err) {
> verbose(env, "kfunc %s#%d reference has not been acquired before\n",
> func_name, meta.func_id);
> @@ -13114,7 +13102,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>
> /* Ensures we don't access the memory after a release_reference() */
> if (meta.ref_obj_id)
> - regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
> + regs[BPF_REG_0].parent_id = meta.ref_obj_id;
>
> if (is_kfunc_rcu_protected(&meta))
> regs[BPF_REG_0].type |= MEM_RCU;
^ permalink raw reply
* [PATCH bpf v2] bpf: Fix NULL pointer dereference in bpf_skb_fib_lookup()
From: Weiming Shi @ 2026-04-23 18:38 UTC (permalink / raw)
To: Martin KaFai Lau, Daniel Borkmann, Alexei Starovoitov,
Andrii Nakryiko, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: John Fastabend, Stanislav Fomichev, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Hao Luo, Jiri Olsa, Simon Horman,
Jesper Dangaard Brouer, bpf, netdev, Xiang Mei, Weiming Shi,
Paul Chaignon
When tot_len is not provided by the user, bpf_skb_fib_lookup()
resolves the FIB result's output device via dev_get_by_index_rcu()
to check skb forwardability and fill in mtu_result. The returned
pointer is dereferenced without a NULL check. If the device is
concurrently unregistered, dev_get_by_index_rcu() returns NULL and
is_skb_forwardable() crashes at dev->flags:
KASAN: null-ptr-deref in range
[0x00000000000000b0-0x00000000000000b7]
Call Trace:
is_skb_forwardable (include/linux/netdevice.h:4365)
bpf_skb_fib_lookup (net/core/filter.c:6446)
bpf_prog_test_run_skb (net/bpf/test_run.c)
__sys_bpf (kernel/bpf/syscall.c)
Add the missing NULL check, returning -ENODEV to be consistent
with how bpf_ipv4_fib_lookup() and bpf_ipv6_fib_lookup() handle
the same condition.
Fixes: 4f74fede40df ("bpf: Add mtu checking to FIB forwarding helper")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Acked-by: Paul Chaignon <paul.chaignon@gmail.com>
---
v2:
Fix Fixes tag: 4f74fede40df, not e1850ea9bd9e (Jiayuan Chen)
Add unlikely() to match bpf_ipv{4,6}_fib_lookup() style (Paul Chaignon)
net/core/filter.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 78b548158fb0..5b4aa9faa707 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6450,6 +6450,8 @@ BPF_CALL_4(bpf_skb_fib_lookup, struct sk_buff *, skb,
* against MTU of FIB lookup resulting net_device
*/
dev = dev_get_by_index_rcu(net, params->ifindex);
+ if (unlikely(!dev))
+ return -ENODEV;
if (!is_skb_forwardable(dev, skb))
rc = BPF_FIB_LKUP_RET_FRAG_NEEDED;
--
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