Netdev List
 help / color / mirror / Atom feed
* [PATCH net v5 3/3] nfc: llcp: fix TLV parsing OOB in nfc_llcp_connect_sn
From: Lekë Hapçiu @ 2026-07-16 20:35 UTC (permalink / raw)
  To: David Heidelberg
  Cc: davem, edumazet, kuba, pabeni, krzk, horms, linux-kernel, netdev,
	oe-linux-nfc, Lekë Hapçiu, stable
In-Reply-To: <20260716203507.7328-1-snowwlake@icloud.com>

nfc_llcp_connect_sn() walks the TLV array of an LLCP CONNECT PDU
looking for the Service Name TLV, but shares the same class of bugs
as nfc_llcp_recv_snl() / nfc_llcp_parse_gb_tlv():

 1. tlv_array_len = skb->len - LLCP_HEADER_SIZE wraps when skb->len
    is 0 or 1.  The subsequent loop then runs far past the buffer.

 2. The per-iteration guard `offset < tlv_array_len` only proves one
    byte is available, but the body reads both tlv[0] (type) and
    tlv[1] (length).

 3. The peer-supplied `length` field is used to advance `tlv` without
    being checked against the remaining array space, so a crafted
    length walks `tlv` past the buffer.  On the following iteration
    tlv[0]/tlv[1] are read from adjacent memory.

 4. When an LLCP_TLV_SN is found, the function returns &tlv[2] with
    *sn_len = length but without verifying that `length` bytes at
    tlv[2..] are still inside the TLV array.  The caller in
    nfc_llcp_recv_connect() then uses this (pointer, length) pair as
    a service name, so it may read past the PDU.

Fix: reject frames smaller than LLCP_HEADER_SIZE up front; add TLV
header and TLV value guards at the top of each iteration.  The value
guard also ensures that the (&tlv[2], length) pair returned on
LLCP_TLV_SN lies fully inside the TLV array.

Also use LLCP_HEADER_SIZE instead of the magic literal `2` to match
the style of neighbouring LLCP receive paths.

Reported-by: Simon Horman <horms@kernel.org>
Closes: https://lore.kernel.org/netdev/20260417160438.GH31784@horms.kernel.org/
Fixes: d646960f7986 ("NFC: Initial LLCP support")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <snowwlake@icloud.com>
---
 net/nfc/llcp_core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index edec2fd83f79..c4dda8e7cfcf 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -849,12 +849,22 @@ static struct nfc_llcp_sock *nfc_llcp_sock_get_sn(struct nfc_llcp_local *local,
 static const u8 *nfc_llcp_connect_sn(const struct sk_buff *skb, size_t *sn_len)
 {
 	u8 type, length;
-	const u8 *tlv = &skb->data[2];
-	size_t tlv_array_len = skb->len - LLCP_HEADER_SIZE, offset = 0;
+	const u8 *tlv;
+	size_t tlv_array_len, offset = 0;
+
+	if (skb->len < LLCP_HEADER_SIZE)
+		return NULL;
+
+	tlv = &skb->data[LLCP_HEADER_SIZE];
+	tlv_array_len = skb->len - LLCP_HEADER_SIZE;
 
 	while (offset < tlv_array_len) {
+		if (tlv_array_len - offset < 2)
+			break;
 		type = tlv[0];
 		length = tlv[1];
+		if (tlv_array_len - offset - 2 < length)
+			break;
 
 		pr_debug("type 0x%x length %d\n", type, length);
 
-- 
2.51.0


^ permalink raw reply related

* Re: [PATCH iproute2-next v7 0/2] rdma: display resource limits in curr/max format
From: patchwork-bot+netdevbpf @ 2026-07-16 20:50 UTC (permalink / raw)
  To: Tao Cui; +Cc: dsahern, leonro, linux-rdma, netdev, cuitao
In-Reply-To: <20260716115237.1859633-1-cui.tao@linux.dev>

Hello:

This series was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Thu, 16 Jul 2026 19:52:35 +0800 you wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> This series adds support for displaying RDMA device resource limits in
> curr/max format in the rdma tool, building on the kernel uapi attribute
> RDMA_NLDEV_ATTR_RES_SUMMARY_ENTRY_MAX which has landed in linux-next
> (kernel commit 5911f6d6e7cc [1]).
> 
> [...]

Here is the summary with links:
  - [iproute2-next,v7,1/2] rdma: update uapi headers
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=39eb3a40d967
  - [iproute2-next,v7,2/2] rdma: display resource limits in curr/max format
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=35a237091c24

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* [PATCH v2 ipsec] xfrm: espintcp: fix UAF during close
From: Sabrina Dubroca @ 2026-07-16 20:54 UTC (permalink / raw)
  To: netdev
  Cc: Steffen Klassert, Herbert Xu, Sabrina Dubroca, Breno Leitao,
	stable, zdi-disclosures

ZDI reported and analyzed a race condition during close for espintcp
sockets:

    espintcp_close() frees emsg->skb via kfree_skb() without holding
    any socket lock. Concurrently, the xfrm_trans_reinject work queue
    invokes esp_output_tcp_finish() -> espintcp_push_skb() ->
    espintcp_push_msgs() -> skb_send_sock_locked(), which reads the
    same skb as a data source.

Fix this by adding a synchronize_rcu() call after resetting sk_prot,
since esp_output_tcp_finish() runs under RCU and won't use a socket
with sk_prot == &tcp_prot.  Simply taking the socket lock in
espintcp_close() could lead to leaks, if esp_output_tcp_finish()
re-adds an skb in the slot we just freed. After this, the existing
barrier() is no longer needed.

Cc: stable@vger.kernel.org
Fixes: e27cca96cd68 ("xfrm: add espintcp (RFC 8229)")
Reported-by: zdi-disclosures@trendmicro.com
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
v2: remove the unnecessary barrier()
v1: https://lore.kernel.org/netdev/50e2ab4348eb8177581058f0152394cfae6a8d27.1783071494.git.sd@queasysnail.net/

 net/xfrm/espintcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
index 374e1b964438..cd817b855ba1 100644
--- a/net/xfrm/espintcp.c
+++ b/net/xfrm/espintcp.c
@@ -515,7 +515,8 @@ static void espintcp_close(struct sock *sk, long timeout)
 	strp_stop(&ctx->strp);
 
 	sk->sk_prot = &tcp_prot;
-	barrier();
+
+	synchronize_rcu();
 
 	disable_work_sync(&ctx->work);
 	strp_done(&ctx->strp);
-- 
2.54.0


^ permalink raw reply related

* [PATCH net-next] net/sched: sch_cake: skip clearing unused tins during rate adjustment
From: Jonas Köppeler @ 2026-07-16 20:59 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Jamal Hadi Salim, Jiri Pirko,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: cake, netdev, linux-kernel, Jonas Köppeler, Mike Pham

When cake_configure_rates() is called from the dequeue path with
rate_adjust=true, it only needs to update the rate parameters. The
loop that clears the unused tins is both unnecessary and harmful in
this path:

 - cake_clear_tin() overwrites q->cur_tin and q->cur_flow, which are
   actively used by cake_dequeue(), corrupting the dequeue state.
 - iterating over the unused tins and their internal queues to purge
   packets adds needless overhead to the hot path.

Skip the entire loop when rate_adjust is set, as neither
cake_clear_tin() nor the mtu_time update are needed when only the
rate changes.

Fixes: 15c2715a5264 ("net/sched: sch_cake: fixup cake_mq rate adjustment for diffserv config")
Signed-off-by: Jonas Köppeler <j.koeppeler@tu-berlin.de>
Tested-by: Mike Pham <mikepham4321@gmail.com>
---
 net/sched/sch_cake.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index f78f8e950776..845e1c017714 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -2609,9 +2609,11 @@ static void cake_configure_rates(struct Qdisc *sch, u64 rate, bool rate_adjust)
 		break;
 	}
 
-	for (c = qd->tin_cnt; c < CAKE_MAX_TINS; c++) {
-		cake_clear_tin(sch, c);
-		qd->tins[c].cparams.mtu_time = qd->tins[ft].cparams.mtu_time;
+	if (!rate_adjust) {
+		for (c = qd->tin_cnt; c < CAKE_MAX_TINS; c++) {
+			cake_clear_tin(sch, c);
+			qd->tins[c].cparams.mtu_time = qd->tins[ft].cparams.mtu_time;
+		}
 	}
 
 	qd->rate_ns   = qd->tins[ft].tin_rate_ns;

---
base-commit: f6f3b36c15ed44de1fbb44e645e4fae8c4a4453e
change-id: 20260716-sch_cake-skip-clearing-tins-856812586cde

Best regards,
--  
Jonas Köppeler <j.koeppeler@tu-berlin.de>


^ permalink raw reply related

* [PATCH] dt-bindings: net: nvidia,tegra234-mgbe: Add missing properties
From: Thierry Reding @ 2026-07-16 21:20 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Jonathan Hunter, netdev, devicetree, linux-tegra,
	linux-kernel

From: Thierry Reding <treding@nvidia.com>

Being a DWMAC derivative, the Tegra234 MGBE supports AXI configuration
nodes named stmmac-axi-config and phandle references to them using the
snps,axi-config property.

While at it, add the 10gbase-r PHY mode.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
This gets rid of the remaining warnings on half of the Tegra234 boards.

 .../devicetree/bindings/net/nvidia,tegra234-mgbe.yaml    | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.yaml b/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.yaml
index 215f14d1897d..dc897e312c55 100644
--- a/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.yaml
+++ b/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.yaml
@@ -81,8 +81,9 @@ properties:
   phy-mode:
     contains:
       enum:
-        - usxgmii
         - 10gbase-kr
+        - 10gbase-r
+        - usxgmii
 
   mdio:
     $ref: mdio.yaml#
@@ -90,6 +91,12 @@ properties:
     description:
       Optional node for embedded MDIO controller.
 
+  snps,axi-config:
+    $ref: snps,dwmac.yaml#/properties/snps,axi-config
+
+  stmmac-axi-config:
+    $ref: snps,dwmac.yaml#/properties/stmmac-axi-config
+
 required:
   - compatible
   - reg
-- 
2.54.0


^ permalink raw reply related

* Re: [PATCH net v2 1/2] vxlan: require CAP_NET_ADMIN in the device netns for changelink
From: Fernando Fernandez Mancera @ 2026-07-16 21:37 UTC (permalink / raw)
  To: Doruk Tan Ozturk
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev, fmancera, sd,
	linville, mschiffer, maoyixie.tju, netdev, linux-kernel, stable
In-Reply-To: <20260716203500.70573-2-doruk@0sec.ai>

On Thu, 16 Jul 2026 22:34:59 +0200, Doruk Tan Ozturk <doruk@0sec.ai> wrote:
> A tunnel changelink() operates on at most two netns, dev_net(dev) and
> the sticky underlay netns vxlan->net. They differ once the device is
> created in or moved to a netns other than the one the request runs in.
> The rtnl changelink path checks CAP_NET_ADMIN only against dev_net(dev),
> so a caller privileged there but not in vxlan->net can rewrite a vxlan
> device whose underlay lives in vxlan->net.
> 
> vxlan_changelink() validates and applies the new configuration against
> vxlan->net (vxlan_config_validate(vxlan->net, ...)) and can reopen the
> underlay socket in that netns, so the same reasoning as the tunnel
> changelink series applies here.
> 
> Gate vxlan_changelink() with rtnl_dev_link_net_capable(), at the top of
> the op before any attribute is parsed, matching ipgre_changelink() and
> the rest of the "require CAP_NET_ADMIN in the device netns for
> changelink" series.
> 
> Found by 0sec automated security-research tooling (https://0sec.ai).
> 
> Fixes: 8bcdc4f3a20b ("vxlan: add changelink support")
> Cc: stable@vger.kernel.org
> Assisted-by: 0sec:multi-model
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
>

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

Thanks!

^ permalink raw reply

* Re: [PATCH net v2 2/2] geneve: require CAP_NET_ADMIN in the device netns for changelink
From: Fernando Fernandez Mancera @ 2026-07-16 21:37 UTC (permalink / raw)
  To: Doruk Tan Ozturk
  Cc: davem, edumazet, kuba, pabeni, andrew+netdev, fmancera, sd,
	linville, mschiffer, maoyixie.tju, netdev, linux-kernel, stable
In-Reply-To: <20260716203500.70573-3-doruk@0sec.ai>

On Thu, 16 Jul 2026 22:35:00 +0200, Doruk Tan Ozturk <doruk@0sec.ai> wrote:
> A tunnel changelink() operates on at most two netns, dev_net(dev) and
> the sticky underlay netns geneve->net. They differ once the device is
> created in or moved to a netns other than the one the request runs in.
> The rtnl changelink path checks CAP_NET_ADMIN only against dev_net(dev),
> so a caller privileged there but not in geneve->net can rewrite a geneve
> device whose underlay lives in geneve->net.
> 
> geneve_changelink() applies the new configuration against geneve->net:
> geneve_link_config() and the geneve_quiesce()/geneve_unquiesce() pair
> reopen the underlay sockets in that netns (geneve_sock_add() uses
> geneve->net), so the same reasoning as the tunnel changelink series
> applies here.
> 
> Gate geneve_changelink() with rtnl_dev_link_net_capable(), at the top of
> the op before any attribute is parsed, matching ipgre_changelink() and
> the rest of the "require CAP_NET_ADMIN in the device netns for
> changelink" series.
> 
> Found by 0sec automated security-research tooling (https://0sec.ai).
> 
> Fixes: 5b861f6baa3a ("geneve: add rtnl changelink support")
> Cc: stable@vger.kernel.org
> Assisted-by: 0sec:multi-model
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
>

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

Thanks!

^ permalink raw reply

* Re: [REGRESSION][BISECTED] stmmac: suspend hangs since 1b9707e6f1a9 ("net: stmmac: enable RPS and RBU interrupts")
From: Andrew Lunn @ 2026-07-16 21:47 UTC (permalink / raw)
  To: tresonic; +Cc: netdev, regressions, rmk+kernel, kuba
In-Reply-To: <7941d239-e5f5-43b5-ae0f-20398221e027@mail.de>

On Thu, Jul 16, 2026 at 10:10:25PM +0200, tresonic wrote:
> Hello,
> Please bear with me, this is my first time writing to a mailing list...

Thanks for the report. Nice description for a first post. Lots of
useful details.

> Since commit 1b9707e6f1a9, suspend (systemctl suspend) causes a full system freeze on my laptop. Fans and keyboard backlight stay powered; the machine is completely unresponsive and requires a hard power-off (holding the power button) to recover. I could not get any kernel output from the hang.

1b9707e6f1a9 makes in effect 4 changes.

Can you do some testing to see if the changes to
DMA_CHAN_INTR_ABNORMAL or the changes to DMA_CHAN_INTR_ABNORMAL_4_10
break it. Or both, but i think that is unlikely.

Once you know which of those is responsible, can you test to see which
of DMA_CHAN_INTR_ENA_RPS or DMA_CHAN_INTR_ENA_RBU broke it.

It kind of sounds like an interrupt storm, but that is just a
guess. If it is an interrupt storm, it suggests an interrupt is not
being disabled during suspend.

   Andrew

^ permalink raw reply

* Re: [PATCH v3 net 4/6] xsk: reclaim invalid multi-buffer Tx descs in ZC path
From: Jason Xing @ 2026-07-16 21:58 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: netdev, bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms,
	bjorn
In-Reply-To: <20260714140722.111645-5-maciej.fijalkowski@intel.com>

On Tue, Jul 14, 2026 at 4:08 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> The zero-copy Tx batch parser stops when it encounters an invalid
> descriptor. If this happens after one or more continuation descriptors,
> the Tx consumer can be advanced past fragments that are neither submitted
> to the driver nor returned to userspace through the completion ring.
>
> A similar problem occurs when a packet exceeds xdp_zc_max_segs. The
> descriptors consumed up to the limit are released without completion, and
> the remaining continuation descriptors can subsequently be interpreted
> as the beginning of another packet.
>
> Parse Tx batches in packet units and distinguish descriptors belonging to
> complete valid packets from descriptors consumed while draining an
> invalid or oversized packet. Return the former to the driver and append
> the latter to the CQ address area so userspace can reclaim their UMEM
> frames.
>
> Once draining starts, continue until the packet's end-of-packet
> descriptor is consumed. Preserve the drain state on the socket when EOP
> has not yet been supplied, so draining can continue during a later call.
> Leave incomplete but otherwise valid packets on the Tx ring. Keep the
> existing handling of standalone invalid descriptors unchanged.
>
> Shared-UMEM pools using multi-buffer Tx also need packet-framed parsing.
> Walk their Tx sockets one packet at a time, preserving the existing
> per-socket fairness scheme, instead of using the legacy one-descriptor
> fallback. Keep that fallback for shared pools that do not use
> multi-buffer Tx. Since the drain state is maintained per socket and both
> the singular and shared paths can resume an interrupted drain, changing
> the socket list from singular to shared requires no special bind-time
> transition.
>
> CQ entries are positional, and drivers may complete only part of the Tx
> work returned by xsk_tx_peek_release_desc_batch(). Therefore, reclaim-only
> entries cannot be published immediately when earlier driver-visible
> descriptors are still outstanding.
>
> Track the number of driver-visible CQ entries preceding the reclaim
> entries. Let xsk_tx_completed() publish partial real Tx completions, and
> publish the reclaim entries only after every earlier Tx descriptor has
> completed. Complete a reclaim-only batch immediately when there is no
> driver-visible work in front of it, and prevent another Tx batch from
> being appended while reclaim entries remain pending.
>
> Also cap batch processing by the size of the pool's temporary descriptor
> array, as Tx rings belonging to sockets sharing a UMEM may have different
> sizes.
>
> This ensures that every descriptor consumed as part of an invalid
> multi-buffer packet is eventually returned to userspace without exposing
> the dropped packet to the driver or violating CQ completion ordering.
>
> Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Thanks for working on this big patch! It's not easy to fix it in a
simpler way, I think. And I didn't observe any obvious performance
impact by xdpsock.

Overall, it looks good to me except for a few minor points:
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>

> ---
>  include/net/xsk_buff_pool.h |   3 +
>  net/xdp/xsk.c               | 192 ++++++++++++++++++++++++++++++++----
>  net/xdp/xsk_buff_pool.c     |   1 +
>  net/xdp/xsk_queue.h         |  76 ++++++++++----
>  4 files changed, 232 insertions(+), 40 deletions(-)
>
> diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> index f5e737a83055..2bb1d122b1bc 100644
> --- a/include/net/xsk_buff_pool.h
> +++ b/include/net/xsk_buff_pool.h
> @@ -78,6 +78,9 @@ struct xsk_buff_pool {
>         u32 chunk_size;
>         u32 chunk_shift;
>         u32 frame_len;
> +       u32 tx_descs_nentries;
> +       u32 reclaim_descs;
> +       u32 tx_zc_pending_descs;
>         u32 xdp_zc_max_segs;
>         u8 tx_metadata_len; /* inherited from umem */
>         u8 cached_need_wakeup;
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 385a3f4a1b32..2909a0ec6837 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -499,6 +499,23 @@ void __xsk_map_flush(struct list_head *flush_list)
>
>  void xsk_tx_completed(struct xsk_buff_pool *pool, u32 nb_entries)
>  {
> +       u32 reclaim_descs = READ_ONCE(pool->reclaim_descs);
> +
> +       if (unlikely(reclaim_descs)) {

Just a side note: it might impact the performance because new descs
need to wait for the existing descs to be completed if there are
reclaim descs.

> +               u32 pending_descs = READ_ONCE(pool->tx_zc_pending_descs);
> +
> +               if (nb_entries < pending_descs) {
> +                       WRITE_ONCE(pool->tx_zc_pending_descs,
> +                                  pending_descs - nb_entries);
> +                       xskq_prod_submit_n(pool->cq, nb_entries);
> +                       return;
> +               }
> +
> +               WRITE_ONCE(pool->tx_zc_pending_descs, 0);
> +               nb_entries += reclaim_descs;
> +               WRITE_ONCE(pool->reclaim_descs, 0);
> +       }
> +
>         xskq_prod_submit_n(pool->cq, nb_entries);
>  }
>  EXPORT_SYMBOL(xsk_tx_completed);
> @@ -574,24 +591,162 @@ static u32 xsk_tx_peek_release_fallback(struct xsk_buff_pool *pool, u32 max_entr
>         return nb_pkts;
>  }
>
> +static void xsk_tx_commit_batch(struct xsk_buff_pool *pool,
> +                               struct xsk_tx_batch *batch)
> +{
> +       u32 nb_descs = xsk_tx_batch_cq_descs(batch);
> +       u32 cq_cached_prod;
> +
> +       if (!nb_descs)
> +               return;
> +
> +       cq_cached_prod = pool->cq->cached_prod;
> +       xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_descs);
> +
> +       if (unlikely(batch->reclaim_descs)) {
> +               u32 cq_pending_descs;
> +
> +               /* CQ is positional. Descriptors already written but not
> +                * submitted must complete before any reclaim-only descriptors
> +                * appended below.
> +                */
> +               cq_pending_descs = cq_cached_prod - xskq_get_prod(pool->cq);
> +
> +               WRITE_ONCE(pool->tx_zc_pending_descs,
> +                          batch->tx_descs + cq_pending_descs);
> +               WRITE_ONCE(pool->reclaim_descs, batch->reclaim_descs);
> +               if (unlikely(!pool->tx_zc_pending_descs))
> +                       xsk_tx_completed(pool, 0);
> +       }
> +}
> +
> +static struct xsk_tx_batch
> +__xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, struct xdp_sock *xs,
> +                                struct xdp_desc *descs, u32 max_descs)
> +{
> +       struct xsk_tx_batch batch = {};
> +       u32 entries;
> +
> +       entries = xskq_cons_nb_entries(xs->tx, max_descs);
> +       if (!entries)
> +               return batch;
> +
> +       batch = xskq_cons_read_desc_batch(xs, pool, descs, max_descs);
> +       if (!xsk_tx_batch_cq_descs(&batch)) {
> +               xs->tx->queue_empty_descs++;
> +               if (batch.consumed_descs) {
> +                       __xskq_cons_release(xs->tx);
> +                       xs->sk.sk_write_space(&xs->sk);
> +               }
> +               return batch;
> +       }
> +
> +       __xskq_cons_release(xs->tx);
> +       xs->sk.sk_write_space(&xs->sk);
> +       return batch;
> +}
> +
> +static struct xsk_tx_batch
> +xsk_tx_peek_release_shared_desc_batch(struct xsk_buff_pool *pool, u32 max_descs)
> +{
> +       u32 cq_descs_before, cq_descs_after;
> +       struct xsk_tx_batch sum_batch = {};
> +       bool budget_exhausted;
> +       u32 per_socket_budget;
> +       struct xdp_sock *xs;
> +
> +       /* The fairness quota must allow one maximum-sized valid packet. */
> +       per_socket_budget = max_t(u32, MAX_PER_SOCKET_BUDGET,
> +                                 pool->xdp_zc_max_segs);
> +
> +again:
> +       budget_exhausted = false;
> +       cq_descs_before = xsk_tx_batch_cq_descs(&sum_batch);
> +       list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list) {
> +               u32 budget, budget_left, offset, remaining;
> +               struct xsk_tx_batch curr_batch;
> +
> +               /* Once reclaim-only descriptors have been appended to the CQ
> +                * address area, do not append driver-visible Tx descriptors
> +                * from another socket after them. xsk_tx_completed() relies on
> +                * all driver-visible descriptors preceding all reclaim-only
> +                * descriptors in CQ order.
> +                */
> +               if (sum_batch.reclaim_descs)
> +                       break;
> +
> +               /* be gentle when playing with pool->tx_descs */

Minor nit: seems unneeded comment?

> +               offset = xsk_tx_batch_cq_descs(&sum_batch);
> +               if (offset >= max_descs)
> +                       break;
> +
> +               if (xs->tx_budget_spent >= per_socket_budget) {
> +                       if (xskq_cons_nb_entries(xs->tx, 1))
> +                               budget_exhausted = true;
> +                       continue;
> +               }
> +
> +               budget_left = per_socket_budget - xs->tx_budget_spent;
> +               remaining = max_descs - offset;
> +               budget = min(remaining, budget_left);
> +
> +               curr_batch = __xsk_tx_peek_release_desc_batch(pool, xs,
> +                                                             pool->tx_descs + offset,
> +                                                             budget);
> +               if (!xsk_tx_batch_cq_descs(&curr_batch)) {
> +                       if (curr_batch.budget_limited && budget_left < remaining)
> +                               budget_exhausted = true;
> +                       xs->tx_budget_spent += curr_batch.consumed_descs;
> +                       continue;
> +               }
> +
> +               xs->tx_budget_spent += curr_batch.consumed_descs;
> +               sum_batch.tx_descs += curr_batch.tx_descs;

No need to use '+' here because of the previous reclaim_descs check.

> +               sum_batch.reclaim_descs += curr_batch.reclaim_descs;
> +       }
> +
> +       cq_descs_after = xsk_tx_batch_cq_descs(&sum_batch);
> +
> +       if (sum_batch.reclaim_descs || cq_descs_after >= max_descs)
> +               return sum_batch;
> +
> +       /* Continue filling the batch while this pass made progress */
> +       if (cq_descs_before != cq_descs_after)
> +               goto again;
> +
> +       if (!budget_exhausted)
> +               return sum_batch;
> +
> +       list_for_each_entry_rcu(xs, &pool->xsk_tx_list, tx_list)
> +               xs->tx_budget_spent = 0;
> +       goto again;
> +}
> +
>  u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
>  {
> +       struct xsk_tx_batch batch = {};
>         struct xdp_sock *xs;
> +       bool umem_shared;
>
>         rcu_read_lock();
> -       if (!list_is_singular(&pool->xsk_tx_list)) {
> -               /* Fallback to the non-batched version */
> -               rcu_read_unlock();
> -               return xsk_tx_peek_release_fallback(pool, nb_pkts);
> -       }
> +       if (unlikely(READ_ONCE(pool->reclaim_descs)))
> +               goto out;
>
> -       xs = list_first_or_null_rcu(&pool->xsk_tx_list, struct xdp_sock, tx_list);
> -       if (!xs) {
> -               nb_pkts = 0;
> +       xs = list_first_or_null_rcu(&pool->xsk_tx_list, struct xdp_sock,
> +                                   tx_list);
> +       if (!xs)
>                 goto out;
> -       }
>
> -       nb_pkts = xskq_cons_nb_entries(xs->tx, nb_pkts);
> +       nb_pkts = min(nb_pkts, pool->tx_descs_nentries);
> +       if (!nb_pkts)
> +               goto out;
> +
> +       umem_shared = !list_is_singular(&pool->xsk_tx_list);
> +
> +       if (umem_shared && !(pool->umem->flags & XDP_UMEM_SG_FLAG)) {
> +               rcu_read_unlock();
> +               return xsk_tx_peek_release_fallback(pool, nb_pkts);
> +       }
>
>         /* This is the backpressure mechanism for the Tx path. Try to
>          * reserve space in the completion queue for all packets, but
> @@ -603,19 +758,16 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
>         if (!nb_pkts)
>                 goto out;
>
> -       nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, nb_pkts);
> -       if (!nb_pkts) {
> -               xs->tx->queue_empty_descs++;
> -               goto out;
> -       }
> -
> -       __xskq_cons_release(xs->tx);
> -       xskq_prod_write_addr_batch(pool->cq, pool->tx_descs, nb_pkts);
> -       xs->sk.sk_write_space(&xs->sk);
> +       batch = umem_shared ?
> +               xsk_tx_peek_release_shared_desc_batch(pool, nb_pkts) :
> +               __xsk_tx_peek_release_desc_batch(pool, xs,
> +                                                pool->tx_descs,
> +                                                nb_pkts);
> +       xsk_tx_commit_batch(pool, &batch);
>
>  out:
>         rcu_read_unlock();
> -       return nb_pkts;
> +       return batch.tx_descs;
>  }
>  EXPORT_SYMBOL(xsk_tx_peek_release_desc_batch);
>
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 12c9fb29af05..a4089480b22b 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -51,6 +51,7 @@ int xp_alloc_tx_descs(struct xsk_buff_pool *pool, struct xdp_sock *xs,
>         if (!pool->tx_descs)
>                 return -ENOMEM;
>
> +       pool->tx_descs_nentries = nentries;
>         return 0;
>  }
>
> diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
> index 3e3fbb73d23e..a15ff1929db6 100644
> --- a/net/xdp/xsk_queue.h
> +++ b/net/xdp/xsk_queue.h
> @@ -58,6 +58,18 @@ struct parsed_desc {
>         u32 valid;
>  };
>
> +struct xsk_tx_batch {
> +       u32 tx_descs;
> +       u32 reclaim_descs;
> +       u32 consumed_descs;
> +       bool budget_limited;
> +};
> +
> +static inline u32 xsk_tx_batch_cq_descs(const struct xsk_tx_batch *batch)
> +{
> +       return batch->tx_descs + batch->reclaim_descs;
> +}
> +
>  /* The structure of the shared state of the rings are a simple
>   * circular buffer, as outlined in
>   * Documentation/core-api/circular-buffers.rst. For the Rx and
> @@ -263,17 +275,18 @@ static inline void parse_desc(struct xsk_queue *q, struct xsk_buff_pool *pool,
>         parsed->mb = xp_mb_desc(desc);
>  }
>
> -static inline
> -u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
> -                             u32 max)
> +static inline struct xsk_tx_batch
> +xskq_cons_read_desc_batch(struct xdp_sock *xs, struct xsk_buff_pool *pool,
> +                         struct xdp_desc *descs, u32 max)
>  {
> -       u32 cached_cons = q->cached_cons, nb_entries = 0;
> -       struct xdp_desc *descs = pool->tx_descs;
> -       u32 total_descs = 0, nr_frags = 0;
> +       bool drain = READ_ONCE(xs->drain_cont);
> +       u32 cached_cons, nb_entries = 0, released;
> +       struct xsk_tx_batch batch = {};
> +       struct xsk_queue *q = xs->tx;
> +       u32 nr_frags = 0;
> +
> +       cached_cons = q->cached_cons;
>
> -       /* track first entry, if stumble upon *any* invalid descriptor, rewind
> -        * current packet that consists of frags and stop the processing
> -        */
>         while (cached_cons != q->cached_prod && nb_entries < max) {
>                 struct xdp_rxtx_ring *ring = (struct xdp_rxtx_ring *)q->ring;
>                 u32 idx = cached_cons & q->ring_mask;
> @@ -282,26 +295,49 @@ u32 xskq_cons_read_desc_batch(struct xsk_queue *q, struct xsk_buff_pool *pool,
>                 descs[nb_entries] = ring->desc[idx];
>                 cached_cons++;
>                 parse_desc(q, pool, &descs[nb_entries], &parsed);
> -               if (unlikely(!parsed.valid))
> -                       break;
> +               if (unlikely(!parsed.valid)) {
> +                       if (!drain && !nr_frags && !parsed.mb)

I understand you're fixing the mb problem here. But I'm wondering if
it still has a problem in the non mb case because the single invalid
packet (mb ==0, nr_frags == 0, drain == 0) that isn't published in CQ
cannot be tracked by application?

My thinking is to just remove the above line in this patch. Or I can
cook a follow-up patch to fix this specific problem?

Thanks,
Jason

> +                               break;
> +
> +                       drain = true;
> +               }
> +
> +               nr_frags++;
> +               nb_entries++;
>
>                 if (likely(!parsed.mb)) {
> -                       total_descs += (nr_frags + 1);
> -                       nr_frags = 0;
> -               } else {
> -                       nr_frags++;
> -                       if (nr_frags == pool->xdp_zc_max_segs) {
> +                       if (unlikely(drain)) {
> +                               batch.reclaim_descs = nr_frags;
> +                               WRITE_ONCE(xs->drain_cont, false);
>                                 nr_frags = 0;
>                                 break;
>                         }
> +
> +                       batch.tx_descs += nr_frags;
> +                       nr_frags = 0;
> +                       continue;
> +               }
> +
> +               if (nr_frags == pool->xdp_zc_max_segs)
> +                       drain = true;
> +       }
> +
> +       if (nr_frags) {
> +               if (drain) {
> +                       batch.reclaim_descs = nr_frags;
> +                       WRITE_ONCE(xs->drain_cont, true);
> +               } else {
> +                       if (nb_entries == max)
> +                               batch.budget_limited = true;
> +                       cached_cons -= nr_frags;
>                 }
> -               nb_entries++;
>         }
>
> -       cached_cons -= nr_frags;
> +       released = cached_cons - q->cached_cons;
>         /* Release valid plus any invalid entries */
> -       xskq_cons_release_n(q, cached_cons - q->cached_cons);
> -       return total_descs;
> +       xskq_cons_release_n(q, released);
> +       batch.consumed_descs = released;
> +       return batch;
>  }
>
>  /* Functions for consumers */
> --
> 2.43.0
>

^ permalink raw reply

* Re: [PATCH net] bpf: tcp: fix double sock release on batch realloc
From: Jordan Rife @ 2026-07-16 23:01 UTC (permalink / raw)
  To: Xiang Mei (Microsoft)
  Cc: Eric Dumazet, Neal Cardwell, Kuniyuki Iwashima, David S . Miller,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netdev, linux-kernel,
	bpf, Martin KaFai Lau, Stanislav Fomichev, AutonomousCodeSecurity,
	tgopinath, kys
In-Reply-To: <20260713233230.3553593-1-xmei5@asu.edu>

On Mon, Jul 13, 2026 at 11:32:30PM +0000, Xiang Mei (Microsoft) wrote:
> bpf_iter_tcp_batch() releases the current batch via
> bpf_iter_tcp_put_batch(), which drops the socket refs and rewrites
> each slot with the socket cookie, then grows the batch. cur_sk/end_sk
> are kept for bpf_iter_tcp_resume(), but on realloc failure the function
> returns ERR_PTR() before resume runs, leaving cur_sk < end_sk over
> slots that now hold cookies rather than sock pointers.
> bpf_iter_tcp_seq_stop() then calls bpf_iter_tcp_put_batch() again and
> dereferences a cookie as a struct sock.
> 
> Empty the batch on the failure path so stop() does not release it
> again. The sockets were already freed by the first
> bpf_iter_tcp_put_batch(), so nothing leaks, and a later read() rescans

Since bpf_iter_tcp_batch returns an ERR_PTR in this case iteration
wouldn't continue on a subsequent read, but otherwise the fix makes
sense to me.

> the bucket from the start instead of skipping it. The sibling
> GFP_NOWAIT failure path still holds real socket references and is left
> for stop() to release.
> 
>   BUG: KASAN: null-ptr-deref in __sock_gen_cookie
>   Read of size 8 at addr 0000000000000059 by task exploit
>    ...
>    __sock_gen_cookie (net/core/sock_diag.c:28)
>    bpf_iter_tcp_put_batch (net/ipv4/tcp_ipv4.c:2918)
>    bpf_iter_tcp_seq_stop (net/ipv4/tcp_ipv4.c:3270)
>    bpf_seq_read (kernel/bpf/bpf_iter.c:205)
>    vfs_read (fs/read_write.c:572)
>    ksys_read (fs/read_write.c:716)
>    do_syscall_64
>    entry_SYSCALL_64_after_hwframe
>   Kernel panic - not syncing: Fatal exception
> 
> Fixes: cdec67a489d4 ("bpf: tcp: Make sure iter->batch always contains a full bucket snapshot")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> ---
>  net/ipv4/tcp_ipv4.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 209ef7522508..dd3ed62704f9 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -3141,8 +3141,11 @@ static struct sock *bpf_iter_tcp_batch(struct seq_file *seq)
>  	bpf_iter_tcp_put_batch(iter);
>  	err = bpf_iter_tcp_realloc_batch(iter, expected * 3 / 2,
>  					 GFP_USER);
> -	if (err)
> +	if (err) {
> +		iter->cur_sk = 0;
> +		iter->end_sk = 0;
>  		return ERR_PTR(err);
> +	}
>  
>  	sk = bpf_iter_tcp_resume(seq);
>  	if (!sk)
> -- 
> 2.43.0
> 

Reviewed-by: Jordan Rife <jordan@jrife.io>

^ permalink raw reply

* Re: [PATCH] dt-bindings: net: nvidia,tegra234-mgbe: Add missing properties
From: Rob Herring (Arm) @ 2026-07-16 23:05 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Eric Dumazet, devicetree, linux-tegra, Andrew Lunn, Paolo Abeni,
	linux-kernel, Jonathan Hunter, Krzysztof Kozlowski,
	David S. Miller, Conor Dooley, Jakub Kicinski, netdev
In-Reply-To: <20260716212001.989872-1-thierry.reding@kernel.org>


On Thu, 16 Jul 2026 23:20:01 +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Being a DWMAC derivative, the Tegra234 MGBE supports AXI configuration
> nodes named stmmac-axi-config and phandle references to them using the
> snps,axi-config property.
> 
> While at it, add the 10gbase-r PHY mode.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> This gets rid of the remaining warnings on half of the Tegra234 boards.
> 
>  .../devicetree/bindings/net/nvidia,tegra234-mgbe.yaml    | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nvidia,tegra234-mgbe.yaml: properties:snps,axi-config: 'anyOf' conditional failed, one must be fixed:
	'description' is a dependency of '$ref'
	'snps,dwmac.yaml#/properties/snps,axi-config' does not match 'types.yaml#\\/definitions\\/'
		hint: A vendor property needs a $ref to types.yaml
	'snps,dwmac.yaml#/properties/snps,axi-config' does not match '^#\\/(definitions|\\$defs)\\/'
		hint: A vendor property can have a $ref to a a $defs schema
	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260716212001.989872-1-thierry.reding@kernel.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply

* Re: [ANN] Google's Netdev-CI for IDPF and GVE
From: Sheena Mohan @ 2026-07-16 23:15 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, andrew+netdev, davem, Eric Dumazet, pabeni, horms,
	Willem de Bruijn, Max Yuan, Pin-yen Lin, Harshitha Ramamurthy,
	Joshua Washington, Danny Gonzalez, David Decotigny, Brian Vazquez
In-Reply-To: <20260619115905.051eb340@kernel.org>

Thank you Jakub. Just wanted to share that we are working on exposing
devlink info for both GVE and IDPF drivers.


On Fri, Jun 19, 2026 at 11:59 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 29 May 2026 13:44:48 -0700 Sheena Mohan wrote:
> > Hi everyone,
> >
> > We are happy to share that Netdev-CI testing on both IDPF (running on
> > Google Bare Metal) and GVE (running on Google Virtual Machines) is now
> > up and running.
> > This NIPA integration work enables executing kselftests against the
> > current proposed net-next kernel branch on real hardware.
> >
> > Thanks to Danny, Max, and Pin-yen for their contributions!
> >
> > The test results and logs are available in:
> >
> > IDPF Results: https://idpf-netdev-nipa.static.usercontent.goog/json/results.json
> > GVE Results: https://gve-netdev-nipa.static.usercontent.goog/json/results.json
>
> Hi Sheena!
>
> The Google runners do not report device info. The results should
> contain a "device" object that identifies external components that
> may cause regressions (like device FW version), see:
> https://github.com/linux-netdev/nipa/wiki/Netdev-CI-system/#device-information
> In practice the main use we currently have for it is to auto-categorize
> the results as executing on a real driver rather than netdevsim.

^ permalink raw reply

* Re: [PATCH v3 06/11] selftests: Fix arm64 IO barriers to match kernel
From: Nathan Chancellor @ 2026-07-16 23:22 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Alex Williamson, David Matlack, Justin Stitt, kvm,
	Leon Romanovsky, linux-kselftest, linux-rdma, llvm, Mark Bloch,
	Bill Wendling, Nick Desaulniers, netdev, Saeed Mahameed,
	Shuah Khan, Tariq Toukan, patches
In-Reply-To: <6-v3-76f117ad04f1+28a90-mlx5st_jgg@nvidia.com>

On Thu, Jul 16, 2026 at 02:03:22PM -0300, Jason Gunthorpe wrote:
> The tools/include readl/writel MMIO accessors on arm64 use
> inner-shareable barriers (dmb ish) while the kernel uses
> outer-shareable (dmb osh).  Fix them to match.
> 
> Add __io_bw() and __io_ar() definitions matching the kernel's
> arch/arm64/include/asm/io.h, including the dummy control dependency
> in __io_ar() that orders MMIO reads against all subsequent
> instructions.
> 
> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
...
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index f02cc8a2e4ae32..fdd22df9174b88 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -5,9 +5,14 @@ ifneq ($(filter %/,$(LLVM)),)
>  LLVM_PREFIX := $(LLVM)
>  else ifneq ($(filter -%,$(LLVM)),)
>  LLVM_SUFFIX := $(LLVM)
> +else ifneq ($(LLVM),1)
> +$(error Invalid value for LLVM, see Documentation/kbuild/llvm.rst)
>  endif
>  
>  CLANG := $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
> +LD := $(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX)
> +# Selftests link through $(CC), so point clang at the LLVM linker.
> +LDFLAGS += --ld-path=$(LD)
>  
>  CLANG_TARGET_FLAGS_arm          := arm-linux-gnueabi
>  CLANG_TARGET_FLAGS_arm64        := aarch64-linux-gnu

Was this hunk intended to be included in this change? It is not
mentioned anywhere. I assume this is the only reason that the LLVM folks
were CC'd on this series.

-- 
Cheers,
Nathan

^ permalink raw reply

* [PATCH net] nfc: llcp: Fix list corruption / refcount desync in nfc_llcp_recv_dm()
From: Aldo Ariel Panzardo @ 2026-07-16 23:26 UTC (permalink / raw)
  To: David Heidelberg, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Simon Horman, netdev, oe-linux-nfc, linux-kernel,
	Aldo Ariel Panzardo

nfc_llcp_recv_dm() handles DM(NOBOUND)/DM(REJ) for a socket that is still
linked on local->connecting_sockets: it looks the socket up with
nfc_llcp_connecting_sock_get(), sets sk->sk_state = LLCP_CLOSED and
returns, without taking the socket lock and without unlinking the socket
from the connecting_sockets list.

llcp_sock_release() selects the list to unlink from by sk_state: a socket
in LLCP_CONNECTING is unlinked from connecting_sockets, otherwise from the
sockets list.  Because recv_dm left the socket physically on
connecting_sockets but in the LLCP_CLOSED state, release() takes the else
branch and calls nfc_llcp_sock_unlink(&local->sockets, sk).  That runs
sk_del_node_init() while holding sockets.lock, i.e. it removes the socket
from the connecting_sockets hlist under the wrong lock.  A concurrent
connect() linking another socket onto connecting_sockets under
connecting_sockets.lock then mutates the same hlist unserialized, which
corrupts the list and desyncs the sk_add_node()/sk_del_node_init()
sock_hold()/__sock_put() pairing.  An unprivileged local process holding
LLCP sockets, with the DM supplied by the remote peer over an established
LLCP link, can drive this to leak kernel sockets without bound (the
mis-decrement goes through the non-freeing __sock_put() path, so the
object is never released), leading to memory exhaustion / DoS.

This is the same class of bug that was fixed in the sibling handler
nfc_llcp_recv_cc() by commit b493ea2765cc ("nfc: llcp: Fix use-after-free
race in nfc_llcp_recv_cc()"); recv_dm did not receive the equivalent fix.

Fix it the same way: take lock_sock(), re-check that the socket is still
hashed (release() may have won the race), and for the NOBOUND/REJ case
unlink it from connecting_sockets before moving it to LLCP_CLOSED.  The
unlink drops the connecting_sockets membership reference via
sk_del_node_init(), leaving the socket unhashed, so the later
nfc_llcp_sock_unlink() in llcp_sock_release() becomes a no-op and no
double put occurs.

Fixes: a69f32af86e3 ("NFC: Socket linked list")
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 net/nfc/llcp_core.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index dc65c719f35f..d8dbb1bb857b 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1249,6 +1249,7 @@ static void nfc_llcp_recv_dm(struct nfc_llcp_local *local,
 	struct nfc_llcp_sock *llcp_sock;
 	struct sock *sk;
 	u8 dsap, ssap, reason;
+	bool connecting = false;
 
 	dsap = nfc_llcp_dsap(skb);
 	ssap = nfc_llcp_ssap(skb);
@@ -1260,6 +1261,7 @@ static void nfc_llcp_recv_dm(struct nfc_llcp_local *local,
 	case LLCP_DM_NOBOUND:
 	case LLCP_DM_REJ:
 		llcp_sock = nfc_llcp_connecting_sock_get(local, dsap);
+		connecting = true;
 		break;
 
 	default:
@@ -1274,10 +1276,33 @@ static void nfc_llcp_recv_dm(struct nfc_llcp_local *local,
 
 	sk = &llcp_sock->sk;
 
+	lock_sock(sk);
+
+	/* Check if socket was destroyed whilst waiting for the lock */
+	if (!sk_hashed(sk)) {
+		release_sock(sk);
+		nfc_llcp_sock_put(llcp_sock);
+		return;
+	}
+
+	/*
+	 * For DM(NOBOUND)/DM(REJ) the socket is still linked on the
+	 * connecting_sockets list.  Unlink it here, under the socket lock,
+	 * before moving it to LLCP_CLOSED: llcp_sock_release() selects the
+	 * list to unlink from by sk_state, so leaving a connecting socket
+	 * in the CLOSED state would make it unlink from the wrong list and
+	 * corrupt the connecting_sockets list / desync the socket refcount.
+	 * This mirrors nfc_llcp_recv_cc().
+	 */
+	if (connecting)
+		nfc_llcp_sock_unlink(&local->connecting_sockets, sk);
+
 	sk->sk_err = ENXIO;
 	sk->sk_state = LLCP_CLOSED;
 	sk->sk_state_change(sk);
 
+	release_sock(sk);
+
 	nfc_llcp_sock_put(llcp_sock);
 }
 

base-commit: 3f1f755366687d051174739fb99f7d560202f60b
-- 
2.43.0


^ permalink raw reply related

* [PATCH] wifi: mwifiex: validate HT/VHT element length before storing beacon IE pointers
From: Christopher Kleiner @ 2026-07-17  0:00 UTC (permalink / raw)
  To: briannorris, linux-wireless; +Cc: francesco, netdev, linux-kernel

mwifiex_update_bss_desc_with_ie() stores raw pointers into the beacon
buffer for the HT Capability, HT Operation, VHT Capability and VHT
Operation elements without checking that the element is long enough to
hold the corresponding fixed-size structure. The generic IE loop only
guarantees that the declared element length fits inside the beacon
buffer (bytes_left >= total_ie_len); it does not guarantee that
element_len is large enough for the struct that later consumers copy.

beacon_buf is a tight kmemdup() of the over-the-air IEs. When the
association command is built, mwifiex_cmd_append_11n_tlv() /
mwifiex_cmd_append_11ac_tlv() copy a fixed number of bytes from the
stored pointers (sizeof(struct ieee80211_ht_cap) and friends). A
malicious AP that emits a beacon or probe response ending in a
truncated (e.g. zero-length) HT Capability element leaves bcn_ht_cap
pointing near the end of the slab, and the subsequent copy reads out of
bounds. The leaked bytes are placed into the association request
transmitted back to the AP, disclosing adjacent slab memory; on
CONFIG_KASAN / panic_on_oops kernels it is an out-of-bounds oops.

Commit 685c9b7750bf ("mwifiex: Abort at too short BSS descriptor
element") added such length checks for the FH/DS/CF/IBSS parameter sets
and a few other elements, but did not cover the HT/VHT capability and
operation elements. Validate element_len against the size of the
structure that will be consumed, mirroring those existing checks.

Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
Cc: stable@vger.kernel.org
Signed-off-by: Christopher Kleiner <chris@kleiner.pro>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 97c0ec3b822e..0196c2adfeed 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1384,6 +1384,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 							bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_HT_CAPABILITY:
+			if (element_len < sizeof(struct ieee80211_ht_cap))
+				return -EINVAL;
 			bss_entry->bcn_ht_cap = (struct ieee80211_ht_cap *)
 					(current_ptr +
 					sizeof(struct ieee_types_header));
@@ -1392,6 +1394,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_HT_OPERATION:
+			if (element_len < sizeof(struct ieee80211_ht_operation))
+				return -EINVAL;
 			bss_entry->bcn_ht_oper =
 				(struct ieee80211_ht_operation *)(current_ptr +
 					sizeof(struct ieee_types_header));
@@ -1400,6 +1404,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_VHT_CAPABILITY:
+			if (element_len < sizeof(struct ieee80211_vht_cap))
+				return -EINVAL;
 			bss_entry->disable_11ac = false;
 			bss_entry->bcn_vht_cap =
 				(void *)(current_ptr +
@@ -1409,6 +1415,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					      bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_VHT_OPERATION:
+			if (element_len < sizeof(struct ieee80211_vht_operation))
+				return -EINVAL;
 			bss_entry->bcn_vht_oper =
 				(void *)(current_ptr +
 					 sizeof(struct ieee_types_header));
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH net-next v3 05/15] ibmveth: Refactor RX interrupt control for MQ RX queues
From: mingming cao @ 2026-07-17  0:17 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
	linuxppc-dev, maddy, mpe, Dave Marquardt
In-Reply-To: <20260714124327.GJ1364329@horms.kernel.org>


On 7/14/26 5:43 AM, Simon Horman wrote:
> On Mon, Jul 06, 2026 at 12:35:53PM -0700, Mingming Cao wrote:
>> Queue 0 and subordinate RX queues use different interrupt control
>> interfaces in PHYP:
>>
>>    - queue 0: h_vio_signal() after h_register_logical_lan()
>>    - queue N: H_VIOCTL against the queue handle/hwirq mapping
>>
>> The current code is single-queue oriented and cannot safely scale to
>> multiple RX queues in poll completion and open/close IRQ setup.
>>
>> Introduce queue-indexed interrupt helpers:
>>
>>    ibmveth_enable_irq(adapter, queue_index)
>>    ibmveth_disable_irq(adapter, queue_index)
>>    ibmveth_setup_rx_interrupts()
>>    ibmveth_cleanup_rx_interrupts()
>>
>> These helpers centralize queue0-vs-subordinate dispatch and make IRQ
>> lifecycle symmetric across open/close and future resize paths.
>>
>> request_irq() is wired with &adapter->napi[i] as dev_id per queue, so
>> interrupt ownership follows the NAPI instance that services that RX
>> queue.
>>
>> Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
>> Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>

**

*Hi, Simon*

*

Thanks for the review of this series!

*

> Although it is added later in this patch set, I believe linux/irqdomain.h
> needs to be included in this patch so that irq_dispose_mapping is defined.
*

Thanks, will fix in v4 — move linux/irqdomain.h into this patch

where irq_dispose_mapping() is first used

*
>
> Also, I think it would be best to add these helpers in the same patch(es)
> that they are first used.  As this will avoid temporal compiler warnings
> about declared but otherwise unused functions. Similarly for patch 13/15.
*

Agreed — will fix in v4. Helpers will be introduced in the same

patch that first uses them (rather than ahead of the open/close

wiring), and I will apply the same rule to patches 13/15.

*
>
> Overall, please make sure that when each patch of the series is applied
> no new warnings or errors are introduced for allmodconfig W=1 builds.
>
> FWIIW, I exercised this using gcc 16.1 from
> https://www.kernel.org/pub/tools/crosstool/
>
> ...
*

Will do — v4 will be checked so each patch applies cleanly under

allmodconfig W=1 (no new warnings/errors), including with a

current crosstool gcc.


Thanks,

Mingming

*


^ permalink raw reply

* Re: [PATCH net-next v3 07/15] ibmveth: Add RX queue register/deregister helpers for MQ
From: mingming cao @ 2026-07-17  0:33 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
	linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260714124338.1831986-1-horms@kernel.org>


On 7/14/26 5:43 AM, Simon Horman wrote:
> 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
> ---
> ibmveth: Add RX queue register/deregister helpers for MQ
>
> This patch introduces new helpers to handle RX queue registration and
> deregistration for MQ support. It replaces the single adapter-level
> register/free paradigm with per-queue register calls to the hypervisor,
> fetching queue handles and IRQs. These new paths are only fully exercised
> when multi-queue mode is enabled.
>
>> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
>> index 63b0184c622a4..7fc11a4e1f61d 100644
>> --- a/drivers/net/ethernet/ibm/ibmveth.c
>> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> [ ... ]
>> @@ -401,6 +401,28 @@ ibmveth_dispose_subordinate_irq_mappings(struct ibmveth_adapter *adapter)
> [ ... ]
>> +static int __maybe_unused
>> +ibmveth_setup_rx_interrupts(struct ibmveth_adapter *adapter)
>> +{
>> +	struct net_device *netdev = adapter->netdev;
>> +	int i, rc;
>> +
>> +	for (i = 0; i < adapter->num_rx_queues; i++) {
>> +		if (!adapter->queue_irq[i]) {
>> +			netdev_err(netdev, "queue %d has invalid IRQ (0)\n", i);
>> +			rc = -EINVAL;
>> +			goto err_free_irqs;
>> +		}
>> +
>> +		rc = request_irq(adapter->queue_irq[i], ibmveth_interrupt,
>> +				 0, netdev->name, &adapter->napi[i]);
>> +		if (rc) {
>> +			netdev_err(netdev,
>> +				   "request_irq() failed for irq 0x%x queue %d: %d\n",
>> +				   adapter->queue_irq[i], i, rc);
>> +			goto err_free_irqs;
>> +		}
>> +	}
>> +
>> +	for (i = 0; i < adapter->num_rx_queues; i++)
>> +		napi_enable(&adapter->napi[i]);
> [Severity: Medium]
> Could this sequence race with packet reception during interface bring-up?
> By calling request_irq() before napi_enable(), the hardware interrupt is
> unmasked while the NAPI state is still NAPI_STATE_DISABLE. If an interrupt
> fires in this window, could ibmveth_interrupt() fail to schedule the NAPI
> poll, potentially leading to dropped events or an interrupt storm?
>
> [ ... ]

**

*On the request_irq vs napi_enable ordering: on open, ibmveth_enable_irq()*

*

(h_vio_signal / H_VIOCTL) runs only after napi_enable(), so PHYP should

not deliver in the request_irq→napi_enable window. That matches what

peers like ibmvnic do (NAPI before firmware unmask). I'll keep the

tested open order (request_irq → napi_enable → PHYP enable) in v4.


For teardown, v4 will follows the hard rule "mask PHYP before

napi_disable": scale-down does that, and close/open-fail cleanup does

too via cleanup_rx_interrupts(). Scale-up still does enable_irq before

napi_enable after publish+replenish — I'm leaving that order for this

v4 and can revisit as a follow-up if needed.

*

>> @@ -1136,6 +1176,293 @@ static void __maybe_unused ibmveth_free_rx_qstats(struct ibmveth_adapter *adapte
> [ ... ]
>> +static int
>> +ibmveth_register_logical_lan_queue(struct ibmveth_adapter *adapter,
>> +				   union ibmveth_buf_desc rxq_desc,
>> +				   int queue_index)
>> +{
> [ ... ]
>> +	if (lpar_rc == H_FUNCTION) {
>> +		if (adapter->multi_queue) {
>> +			netdev_info(adapter->netdev,
>> +				    "Multi queue mode not supported by firmware, falling back to single queue\n");
>> +			adapter->multi_queue = 0;
>> +		} else {
>> +			netdev_err(adapter->netdev,
>> +				   "Unexpected H_FUNCTION for queue %d registration (MQ mode already disabled)\n",
>> +				   queue_index);
>> +		}
>> +		return lpar_rc;
> [Severity: Medium]
> Does returning lpar_rc here cause a regression for the single queue fallback?
> Since this error propagates up to ibmveth_register_rx_queues(), it seems it
> will unconditionally jump to err_unregister and fail the interface open
> rather than allowing it to continue with a single queue.
*

Good catch — clearing multi_queue still returned the

error and failed open, so the "falling back to single queue" log was

misleading. In v4 I'll treat subordinate H_FUNCTION as a hard error:

honest H_FUNCTION-specific netdev_err, then the existing generic logs,

without clearing multi_queue or claiming fallback.

*
>> +	}
>> +
>> +	if (try_again) {
>> +		try_again = 0;
>> +		goto retry;
> [Severity: Low]
> Does this retry loop provide any benefit? Unlike the primary queue which
> calls h_free_logical_lan() before retrying to clear stale state, this
> jumps back to retry the exact same hypercall with the exact same arguments,
> which seems like it will immediately fail again.
*

The subordinate try_again path was indeed empty logic. Queue 0 retries

only after h_free_logical_lan() clears stale PHYP state (kexec). The

subqueue path retried the same H_REG_LOGICAL_LAN_QUEUE with the same

args and no free, so the second attempt could not succeed for any reason

we implemented — and H_FUNCTION already returned before the retry.

In v4 I'll drop that blind retry and report the failure once per queue.

*
>> +	}
> [ ... ]
>> +static int
>> +ibmveth_register_rx_queues(struct ibmveth_adapter *adapter, u64 mac_address)
>> +{
> [ ... ]
>> +	for (i = 1; i < adapter->num_rx_queues; i++) {
>> +		rc = ibmveth_register_single_rx_queue(adapter, i, mac_address);
>> +		if (rc) {
>> +			if (!adapter->queue_handle[i] || !adapter->queue_irq[i]) {
>> +				netdev_err(netdev,
>> +					   "Invalid hypervisor return for queue %d: handle=0x%llx irq=%u\n",
>> +					   i, adapter->queue_handle[i],
>> +					   adapter->queue_irq[i]);
>> +			}
> [Severity: Low]
> Is this validation check placed correctly? Because it sits inside the
> if (rc) error block, it will log "Invalid hypervisor return" whenever
> registration fails normally (e.g., due to resource constraints). It appears
> the hypervisor is not expected to populate the handle and irq on failure.

**

*Good catch. on ordinary register failure handle/irq are still zero, so*

*

that caller message falsely claimed an invalid hypervisor return and

duplicated ibmveth_register_single_rx_queue() (which already logs the

hcall rc). In v4 I'll drop that extra log at the caller.


Thanks for the reviewing!

Mingming

*

>
>> +			goto err_unregister;
>> +		}
>> +	}

^ permalink raw reply

* Re: [PATCH v3 06/11] selftests: Fix arm64 IO barriers to match kernel
From: Jason Gunthorpe @ 2026-07-17  0:36 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Alex Williamson, David Matlack, Justin Stitt, kvm,
	Leon Romanovsky, linux-kselftest, linux-rdma, llvm, Mark Bloch,
	Bill Wendling, Nick Desaulniers, netdev, Saeed Mahameed,
	Shuah Khan, Tariq Toukan, patches
In-Reply-To: <20260716232210.GA700430@ax162>

On Thu, Jul 16, 2026 at 04:22:10PM -0700, Nathan Chancellor wrote:
> On Thu, Jul 16, 2026 at 02:03:22PM -0300, Jason Gunthorpe wrote:
> > The tools/include readl/writel MMIO accessors on arm64 use
> > inner-shareable barriers (dmb ish) while the kernel uses
> > outer-shareable (dmb osh).  Fix them to match.
> > 
> > Add __io_bw() and __io_ar() definitions matching the kernel's
> > arch/arm64/include/asm/io.h, including the dummy control dependency
> > in __io_ar() that orders MMIO reads against all subsequent
> > instructions.
> > 
> > Assisted-by: Claude:claude-opus-4.6
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ...
> > diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> > index f02cc8a2e4ae32..fdd22df9174b88 100644
> > --- a/tools/testing/selftests/lib.mk
> > +++ b/tools/testing/selftests/lib.mk
> > @@ -5,9 +5,14 @@ ifneq ($(filter %/,$(LLVM)),)
> >  LLVM_PREFIX := $(LLVM)
> >  else ifneq ($(filter -%,$(LLVM)),)
> >  LLVM_SUFFIX := $(LLVM)
> > +else ifneq ($(LLVM),1)
> > +$(error Invalid value for LLVM, see Documentation/kbuild/llvm.rst)
> >  endif
> >  
> >  CLANG := $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
> > +LD := $(LLVM_PREFIX)ld.lld$(LLVM_SUFFIX)
> > +# Selftests link through $(CC), so point clang at the LLVM linker.
> > +LDFLAGS += --ld-path=$(LD)
> >  
> >  CLANG_TARGET_FLAGS_arm          := arm-linux-gnueabi
> >  CLANG_TARGET_FLAGS_arm64        := aarch64-linux-gnu
> 
> Was this hunk intended to be included in this change? It is not
> mentioned anywhere. I assume this is the only reason that the LLVM folks
> were CC'd on this series.

Er no, I can no longer remember what that was about, it wasn't in
v2. Actually I also see I forgot to delete this patch as Will
suggested in the first place.

Jason

^ permalink raw reply

* Re: [PATCH net-next v3 08/15] ibmveth: Refactor open/close into MQ-ready resource pipeline
From: mingming cao @ 2026-07-17  0:53 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
	linuxppc-dev, maddy, mpe, Dave Marquardt
In-Reply-To: <20260714124736.GK1364329@horms.kernel.org>

On 7/14/26 5:47 AM, Simon Horman wrote:

> On Mon, Jul 06, 2026 at 12:35:56PM -0700, Mingming Cao wrote:
>
> ...
>
>> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> ...
*

Hi, Simon


Thanks for the careful read on open/close — these were all fair points.

*
>>   /**
>>    * ibmveth_register_logical_lan_queue - Register subordinate queue with hypervisor
>>    * @adapter: ibmveth adapter structure
>> @@ -1466,208 +1479,108 @@ ibmveth_register_rx_queues(struct ibmveth_adapter *adapter, u64 mac_address)
>>   static int ibmveth_open(struct net_device *netdev)
>>   {
>>   	struct ibmveth_adapter *adapter = netdev_priv(netdev);
>> -	u64 mac_address;
>> +	u64 mac_address = ether_addr_to_u64(netdev->dev_addr);
>>   	int rxq_entries = 1;
>> -	unsigned long lpar_rc;
>>   	int rc;
>> -	union ibmveth_buf_desc rxq_desc;
>>   	int i;
>> -	struct device *dev;
>>   
>>   	netdev_dbg(netdev, "open starting\n");
>>   
>> -	napi_enable(&adapter->napi[0]);
>> -
>> -	for(i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
>> +	for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
>>   		rxq_entries += adapter->rx_buff_pool[0][i].size;
>>   
>> -	rc = -ENOMEM;
>> -	adapter->buffer_list_addr[0] = (void *)get_zeroed_page(GFP_KERNEL);
>> -	if (!adapter->buffer_list_addr[0]) {
>> -		netdev_err(netdev, "unable to allocate list pages\n");
>> +	rc = ibmveth_alloc_rx_qstats(adapter);
>> +	if (rc)
>>   		goto out;
>> -	}
>>   
>> -	adapter->filter_list_addr = (void*) get_zeroed_page(GFP_KERNEL);
>> -	if (!adapter->filter_list_addr) {
>> -		netdev_err(netdev, "unable to allocate filter pages\n");
>> -		goto out_free_buffer_list;
>> -	}
>> -
>> -	dev = &adapter->vdev->dev;
>> +	rc = ibmveth_alloc_filter_list(adapter);
>> +	if (rc)
>> +		goto out_free_rx_qstats;
>>   
>> -	adapter->rx_queue[0].queue_len = sizeof(struct ibmveth_rx_q_entry) *
>> -						rxq_entries;
>> -	adapter->rx_queue[0].queue_addr =
>> -		dma_alloc_coherent(dev, adapter->rx_queue[0].queue_len,
>> -				   &adapter->rx_queue[0].queue_dma, GFP_KERNEL);
>> -	if (!adapter->rx_queue[0].queue_addr)
>> +	rc = ibmveth_alloc_rx_queues(adapter, rxq_entries);
>> +	if (rc)
>>   		goto out_free_filter_list;
>>   
>> -	adapter->buffer_list_dma[0] = dma_map_single(dev,
>> -						     adapter->buffer_list_addr[0],
>> -						     4096, DMA_BIDIRECTIONAL);
>> -	if (dma_mapping_error(dev, adapter->buffer_list_dma[0])) {
>> -		netdev_err(netdev, "unable to map buffer list pages\n");
>> +	rc = ibmveth_alloc_buffer_pools(adapter);
>> +	if (rc)
>>   		goto out_free_queue_mem;
>> -	}
>>   
>> -	adapter->filter_list_dma = dma_map_single(dev,
>> -			adapter->filter_list_addr, 4096, DMA_BIDIRECTIONAL);
>> -	if (dma_mapping_error(dev, adapter->filter_list_dma)) {
>> -		netdev_err(netdev, "unable to map filter list pages\n");
>> -		goto out_unmap_buffer_list;
>> -	}
>> +	rc = ibmveth_register_rx_queues(adapter, mac_address);
>> +	if (rc)
>> +		goto out_free_buffer_pools;
>>   
>> -	for (i = 0; i < netdev->real_num_tx_queues; i++) {
>> -		if (ibmveth_allocate_tx_ltb(adapter, i))
>> -			goto out_free_tx_ltb;
>> +	rc = netif_set_real_num_rx_queues(netdev, adapter->num_rx_queues);
>> +	if (rc) {
>> +		netdev_err(netdev, "failed to set number of rx queues\n");
>> +		goto out_unregister_queues;
>>   	}
>>   
>> -	adapter->rx_queue[0].index = 0;
>> -	adapter->rx_queue[0].num_slots = rxq_entries;
>> -	adapter->rx_queue[0].toggle = 1;
>> -
>> -	mac_address = ether_addr_to_u64(netdev->dev_addr);
>> -
>> -	rxq_desc.fields.flags_len = IBMVETH_BUF_VALID |
>> -					adapter->rx_queue[0].queue_len;
>> -	rxq_desc.fields.address = adapter->rx_queue[0].queue_dma;
>> -
>> -	netdev_dbg(netdev, "buffer list @ 0x%p\n", adapter->buffer_list_addr[0]);
>> -	netdev_dbg(netdev, "filter list @ 0x%p\n", adapter->filter_list_addr);
>> -	netdev_dbg(netdev, "receive q   @ 0x%p\n", adapter->rx_queue[0].queue_addr);
>> -
>> -	h_vio_signal(adapter->vdev->unit_address, VIO_IRQ_DISABLE);
>> -
>> -	lpar_rc = ibmveth_register_logical_lan(adapter, rxq_desc, mac_address);
>> -
>> -	if (lpar_rc != H_SUCCESS) {
>> -		netdev_err(netdev, "h_register_logical_lan failed with %ld\n",
>> -			   lpar_rc);
>> -		netdev_err(netdev, "buffer TCE:0x%llx filter TCE:0x%llx rxq "
>> -			   "desc:0x%llx MAC:0x%llx\n",
>> -				     adapter->buffer_list_dma[0],
>> -				     adapter->filter_list_dma,
>> -				     rxq_desc.desc,
>> -				     mac_address);
>> -		rc = -ENONET;
>> -		goto out_unmap_filter_list;
>> -	}
>> +	rc = ibmveth_setup_rx_interrupts(adapter);
>> +	if (rc)
>> +		goto out_unregister_queues;
>>   
>> -	for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) {
>> -		if (!adapter->rx_buff_pool[0][i].active)
>> -			continue;
>> -		if (ibmveth_alloc_buffer_pool(&adapter->rx_buff_pool[0][i])) {
>> -			netdev_err(netdev, "unable to alloc pool\n");
>> -			adapter->rx_buff_pool[0][i].active = 0;
>> -			rc = -ENOMEM;
>> -			goto out_free_buffer_pools;
>> +	if (adapter->num_rx_queues > 1) {
>> +		for (i = 0; i < adapter->num_rx_queues; i++) {
>> +			netdev_dbg(netdev, "initial replenish cycle for queue %d\n", i);
>> +			ibmveth_replenish_task(adapter, i);
> ibmveth_replenish_task() only has one parameter
> until a later patch in this series.
*

Agreed. Will address this In v4.

the multi-arg replenish call moves to the patch that introduces

the queue-index parameter (or that patch lands

before this open() wiring).

*
>>   		}
>> +	} else {
>> +		netdev_dbg(netdev, "initial replenish cycle\n");
>> +		ibmveth_interrupt(adapter->queue_irq[0], &adapter->napi[0]);
>>   	}
>>   
>> -	netdev_dbg(netdev, "registering irq 0x%x\n", netdev->irq);
>> -	rc = request_irq(netdev->irq, ibmveth_interrupt, 0, netdev->name,
>> -			 netdev);
>> -	if (rc != 0) {
>> -		netdev_err(netdev, "unable to request irq 0x%x, rc %d\n",
>> -			   netdev->irq, rc);
>> -		do {
>> -			lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
>> -		} while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
>> -
>> -		goto out_free_buffer_pools;
>> -	}
>> -
>> -	rc = -ENOMEM;
>> -
>> -	netdev_dbg(netdev, "initial replenish cycle\n");
>> -	ibmveth_interrupt(netdev->irq, netdev);
>> +	rc = ibmveth_alloc_tx_resources(adapter);
>> +	if (rc)
>> +		goto out_cleanup_rx_interrupts;
>>   
>>   	netif_tx_start_all_queues(netdev);
>>   
>>   	netdev_dbg(netdev, "open complete\n");
>> -
>>   	return 0;
>>   
>> +out_cleanup_rx_interrupts:
>> +	ibmveth_cleanup_rx_interrupts(adapter);
>> +out_free_tx_resources:
>> +	ibmveth_free_tx_resources(adapter);
> The out_free_tx_resources label is unused until a later patch of this
> series, so it should be added in that patch rather than this one.
*

Agreed — will introduce out_free_tx_resources only when a caller

needs that jump target.

*
>
> And it's not clear to me that ibmveth_free_tx_resources() should
> be called when jumping to out_cleanup_rx_interrupts as
> in that case ibmveth_alloc_tx_resources() hasn't run successfully.
>
> The AI-generated review on sashiko.dev also highlights the error handling
> here:
>
>   "Are the unwind labels ordered incorrectly here?
>
>   "If ibmveth_setup_rx_interrupts() fails, it jumps to
>    out_unregister_queues, which is placed after out_free_buffer_pools. Does
>    this mean we skip freeing the buffer pools and leak memory?
*

Yes — those gotos currently skip free_buffer_pools(). Will fix

unwind ordering in v4 so buffer pools are always released.

*

>
>   "Also, if ibmveth_alloc_tx_resources() fails, it internally frees
>    partially allocated LTBs. It then jumps to out_cleanup_rx_interrupts and
>    falls through to out_free_tx_resources. Because ibmveth_free_tx_ltb()
>    calls dma_unmap_single() unconditionally without checking or zeroing
>    tx_ltb_dma, will this cause a double free and an invalid DMA unmap?

**

*Correct — on alloc_tx_resources() failure TX is already partially*

*

cleaned inside the helper, so falling through to

free_tx_resources() is wrong and can double-unmap. Will fix the

open() unwind graph in v4.

*

>
>   "Finally, in the fall-through path from out_cleanup_rx_interrupts,
>    out_free_buffer_pools is executed before out_unregister_queues (which
>    calls ibmveth_free_all_queues() to unregister the logical LAN). Does this
>    free and unmap the RX buffers while the hypervisor's logical LAN is still
>    active, potentially allowing the hypervisor to DMA incoming packets into
>    freed memory?
>
**

*Agreed — freeing RX pools before h_free_logical_lan() is unsafe.*

*

v4 will unregister/free the logical LAN before releasing buffer

pools on both open failure and close

**

*Thanks,*

*

Mingming

*

*

>>   out_free_buffer_pools:
>> -	while (--i >= 0) {
>> -		if (adapter->rx_buff_pool[0][i].active)
>> -			ibmveth_free_buffer_pool(adapter,
>> -						 &adapter->rx_buff_pool[0][i]);
>> -	}
>> -out_unmap_filter_list:
>> -	dma_unmap_single(dev, adapter->filter_list_dma, 4096,
>> -			 DMA_BIDIRECTIONAL);
>> -
>> -out_free_tx_ltb:
>> -	while (--i >= 0) {
>> -		ibmveth_free_tx_ltb(adapter, i);
>> -	}
>> -
>> -out_unmap_buffer_list:
>> -	dma_unmap_single(dev, adapter->buffer_list_dma[0], 4096,
>> -			 DMA_BIDIRECTIONAL);
>> +	ibmveth_free_buffer_pools(adapter);
>> +out_unregister_queues:
>> +	ibmveth_dispose_subordinate_irq_mappings(adapter);
>> +	ibmveth_free_all_queues(adapter);
>>   out_free_queue_mem:
>> -	dma_free_coherent(dev, adapter->rx_queue[0].queue_len,
>> -			  adapter->rx_queue[0].queue_addr,
>> -			  adapter->rx_queue[0].queue_dma);
>> +	ibmveth_cleanup_rx_resources(adapter);
>>   out_free_filter_list:
>> -	free_page((unsigned long)adapter->filter_list_addr);
>> -out_free_buffer_list:
>> -	free_page((unsigned long)adapter->buffer_list_addr[0]);
>> +	ibmveth_free_filter_list(adapter);
>> +out_free_rx_qstats:
>> +	ibmveth_free_rx_qstats(adapter);
>>   out:
>> -	napi_disable(&adapter->napi[0]);
>>   	return rc;
>>   }
> ...

^ permalink raw reply

* Re: [PATCH] rds: use krealloc_array() for iovector growth
From: Allison Henderson @ 2026-07-17  1:00 UTC (permalink / raw)
  To: Weimin Xiong
  Cc: netdev, linux-rdma, rds-devel, linux-kernel, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman
In-Reply-To: <20260716025101.118159-1-xiongwm2026@163.com>

On Thu, 2026-07-16 at 10:51 +0800, Weimin Xiong wrote:
> Use krealloc_array() for growing the RDS iovector array. This makes the
> array allocation overflow-safe and derives the element size from the
> array pointer.
> 
> Signed-off-by: Weimin Xiong <xiongwm2026@163.com>

Hi Weinmin,

Thanks for working on this.  This looks mostly good to me, the conversion
looks correct.  Just one nit, since this is a hardening fix
and not a bug fix, it should target the net-next branch in the subject
line like this:

[PATCH net-next v2] net/rds: use krealloc_array() for iovector growth

With that fixed, you can add my rvb:
Reviewed-by: Allison Henderson <achender@kernel.org>

Thanks!
Allison

> ---
> diff --git a/net/rds/send.c b/net/rds/send.c
> index 68be1bf0e..6a567c97a 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -971,11 +971,8 @@ static int rds_rm_size(struct msghdr *msg, int num_sgs,
>  				return -EINVAL;
>  			if (vct->indx >= vct->len) {
>  				vct->len += vct->incr;
> -				tmp_iov =
> -					krealloc(vct->vec,
> -						 vct->len *
> -						 sizeof(struct rds_iov_vector),
> -						 GFP_KERNEL);
> +				tmp_iov = krealloc_array(vct->vec, vct->len,
> +							 sizeof(*vct->vec), GFP_KERNEL);
>  				if (!tmp_iov) {
>  					vct->len -= vct->incr;
>  					return -ENOMEM;
> 


^ permalink raw reply

* Re: [PATCH net-next v3 09/15] ibmveth: Add queue-aware RX buffer submit helper for MQ
From: mingming cao @ 2026-07-17  1:02 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
	linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260714125019.1833862-2-horms@kernel.org>

On 7/14/26 5:50 AM, Simon Horman wrote:

> 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
> ---
> ibmveth: Add queue-aware RX buffer submit helper for MQ
>
> This patch updates the ibmveth driver's buffer replenishment path to
> be queue-aware for multi-queue receive support. It adds a helper to select
> the correct hypervisor call and handles fallback to single-buffer mode if
> batch operations are unsupported.
>
>> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
>> index fa2d4777ffc7b..b3b3886c3eedb 100644
>> --- a/drivers/net/ethernet/ibm/ibmveth.c
>> +++ b/drivers/net/ethernet/ibm/ibmveth.c
>> @@ -597,11 +597,73 @@ static inline void ibmveth_flush_buffer(void *addr, unsigned long length)
> [ ... ]
>
>> +static long ibmveth_add_logical_lan_buffers(struct ibmveth_adapter *adapter,
>> +					    union ibmveth_buf_desc *descs,
>> +					    int filled,
>> +					    unsigned long buff_size,
>> +					    int queue_index)
>> +{
>> +	struct vio_dev *vdev = adapter->vdev;
>> +	unsigned long rc;
>> +
>> +	if (adapter->multi_queue) {
>> +		unsigned long buffersznum = (buff_size << 32) | filled;
>> +		unsigned long ioba[IBMVETH_MAX_RX_PER_HCALL / 2] = {0};
>> +		int i;
>> +
>> +		/* Pack descriptor addresses into ioba pairs.
>> +		 * Each ioba holds two 32-bit addresses packed into 64 bits:
>> +		 * - Even descriptors (0,2,4...) go in high 32 bits
>> +		 * - Odd descriptors (1,3,5...) go in low 32 bits
>> +		 */
>> +		for (i = 0; i < filled && i < IBMVETH_MAX_RX_PER_HCALL; i++) {
>> +			int pair_idx = i / 2;           /* Which pair: 0-5 */
>> +			int is_high = (i % 2 == 0);     /* High or low 32 bits */
>> +
>> +			if (is_high)
>> +				ioba[pair_idx] = (unsigned long)descs[i].fields.address << 32;
>> +			else
>> +				ioba[pair_idx] |= descs[i].fields.address;
>> +		}
>> +
>> +		rc = h_add_logical_lan_buffers_queue(vdev->unit_address,
>> +						     adapter->queue_handle[queue_index],
>> +						     buffersznum,
>> +						     ioba[0], ioba[1], ioba[2],
>> +						     ioba[3], ioba[4], ioba[5]);
>> +		adapter->hcall_stats.add_bufs_queue++;
> [Severity: Medium]
> Will this counter increment lose updates when multiple NAPI poll routines run
> concurrently on different CPUs?
*

Thanks — these are informational ethtool/debug counters and can

race under concurrent NAPI, as with many driver stats. Datapath

correctness does not depend on them.

*
> Since multi-queue enables concurrent execution, it seems global statistic
> counters like hcall_stats.add_bufs_queue and replenish_add_buff_success might
> need atomic or per-CPU operations to avoid read-modify-write data races.
*

  I'm happy to harden selected counters with atomic64_t (or percpu counter)

in a follow-up; I don't think that needs to gate MQ...

*

>> +	} else if (filled == 1) {
>> +		rc = h_add_logical_lan_buffer(vdev->unit_address,
>> +					      descs[0].desc);
>> +		adapter->hcall_stats.add_buf++;
>> +	} else {
>> +		rc = h_add_logical_lan_buffers(vdev->unit_address,
>> +					       descs[0].desc, descs[1].desc,
>> +					       descs[2].desc, descs[3].desc,
>> +					       descs[4].desc, descs[5].desc,
>> +					       descs[6].desc, descs[7].desc);
>> +		adapter->hcall_stats.add_bufs++;
>> +	}
>> +
>> +	return rc;
>> +}
> [ ... ]
>
>> @@ -745,24 +798,19 @@ static void ibmveth_replenish_buffer_pool(struct ibmveth_adapter *adapter,
>>   		}
>>   		adapter->replenish_add_buff_failure += filled;
>>   
>> -		/*
>> -		 * If multi rx buffers hcall is no longer supported by FW
>> -		 * e.g. in the case of Live Partition Migration
>> -		 */
>> -		if (batch > 1 && lpar_rc == H_FUNCTION) {
>> -			/*
> [ ... ]
>
>> +		if (lpar_rc == H_FUNCTION) {
>> +			if (adapter->multi_queue) {
>> +				netdev_err(adapter->netdev,
>> +					   "Unexpected H_FUNCTION from multi-queue buffer add (queue=%d, batch=%d)\n",
>> +					   queue_index, batch);
>> +				break;
>> +			} else if (batch > 1) {
>> +				netdev_warn(adapter->netdev,
>> +					    "H_FUNCTION from legacy batch buffer add (batch=%d), falling back to single buffer mode\n",
>> +					    batch);
>> +				adapter->rx_buffers_per_hcall = 1;
>> +				continue;
> [Severity: High]
> Does this continue statement cause an infinite loop?
*Good catch! yes.*
>
> Looking at ibmveth_replenish_buffer_pool(), the local batch variable is set
> before the while loop:
>
> 	batch = adapter->rx_buffers_per_hcall;
> 	while (remaining > 0) {
> 		...
>
> If the fallback updates adapter->rx_buffers_per_hcall to 1 and issues a continue,
> the loop will restart but the local batch variable will retain its original
> value.
>
> Would this lead to the loop repeatedly attempting the unsupported batch size
> and failing with H_FUNCTION forever?
You are correct, the MQ replenish rewrite accidentally used continue 
instead of break on the non-MQ multi-buffer path (!multi_queue && batch 
 > 1 && H_FUNCTION). The MQ H_FUNCTION arm already hard-fails and*

breaks — that wasn't the bug. I'll restore the pre-MQ break in v4.

*
>> +			}
>>   		}
>>   		break;
>>   	}
>> @@ -784,18 +832,24 @@ static void ibmveth_update_rx_no_buffer(struct ibmveth_adapter *adapter)
>>   }
>>   
>>   /* replenish routine */
>> -static void ibmveth_replenish_task(struct ibmveth_adapter *adapter)
>> +static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
>> +				   int queue_index)
>>   {
>>   	int i;
>>   
>> +	if (queue_index >= adapter->num_rx_queues)
>> +		return;
>> +
>>   	adapter->replenish_task_cycles++;
> [Severity: Medium]
> Similar to the hcall_stats counters above, could this increment suffer from data
> races when multiple queues are processed concurrently across different CPUs?Ah yes
Ah yes, *

Same as the hcall_stats note — racy informational counter; optional

**

atomic/per-queue follow-up, not a functional MQ bug. (since this series 
is already large enough)

*

Thanks for your review again!
Mingming

^ permalink raw reply

* [PATCH net v2] i40e: xsk: fix multi-buffer XDP_PASS skb construction
From: Chenguang Zhao @ 2026-07-17  1:24 UTC (permalink / raw)
  To: anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev, davem,
	edumazet, kuba, pabeni
  Cc: intel-wired-lan, netdev, chenguang.zhao, Chenguang Zhao

From: Chenguang Zhao <zhaochenguang@kylinos.cn>

When AF_XDP ZC receives a multi-buffer frame and XDP returns XDP_PASS,
i40e_construct_skb_zc() copied frags incorrectly: memcpy used
skb_frag_page() (page metadata) and __skb_fill_page_desc_noacc() was
given a virtual address instead of a struct page *.

Drop the custom helper and use xdp_build_skb_from_zc() instead. On
failure, free the xdp buff in the caller. Push the Ethernet header
back before eth_skb_pad()/i40e_process_skb_fields() because
xdp_build_skb_from_zc() already called eth_type_trans().

Fixes: 1c9ba9c14658 ("i40e: xsk: add RX multi-buffer support")
Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn>
---
Revised as suggested by Maciej:
 - Replace i40e_construct_skb_zc() with xdp_build_skb_from_zc()

v1:
 https://lore.kernel.org/all/20260714025112.284724-1-chenguang.zhao@linux.dev/

 drivers/net/ethernet/intel/i40e/i40e_xsk.c | 73 +++-------------------
 1 file changed, 8 insertions(+), 65 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 9f47388eaba5..1319a5c22625 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -3,6 +3,7 @@
 
 #include <linux/bpf_trace.h>
 #include <linux/unroll.h>
+#include <net/xdp.h>
 #include <net/xdp_sock_drv.h>
 #include "i40e_txrx_common.h"
 #include "i40e_xsk.h"
@@ -277,70 +278,6 @@ bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 count)
 	return count == nb_buffs;
 }
 
-/**
- * i40e_construct_skb_zc - Create skbuff from zero-copy Rx buffer
- * @rx_ring: Rx ring
- * @xdp: xdp_buff
- *
- * This functions allocates a new skb from a zero-copy Rx buffer.
- *
- * Returns the skb, or NULL on failure.
- **/
-static struct sk_buff *i40e_construct_skb_zc(struct i40e_ring *rx_ring,
-					     struct xdp_buff *xdp)
-{
-	unsigned int totalsize = xdp->data_end - xdp->data_meta;
-	unsigned int metasize = xdp->data - xdp->data_meta;
-	struct skb_shared_info *sinfo = NULL;
-	struct sk_buff *skb;
-	u32 nr_frags = 0;
-
-	if (unlikely(xdp_buff_has_frags(xdp))) {
-		sinfo = xdp_get_shared_info_from_buff(xdp);
-		nr_frags = sinfo->nr_frags;
-	}
-	net_prefetch(xdp->data_meta);
-
-	/* allocate a skb to store the frags */
-	skb = napi_alloc_skb(&rx_ring->q_vector->napi, totalsize);
-	if (unlikely(!skb))
-		goto out;
-
-	memcpy(__skb_put(skb, totalsize), xdp->data_meta,
-	       ALIGN(totalsize, sizeof(long)));
-
-	if (metasize) {
-		skb_metadata_set(skb, metasize);
-		__skb_pull(skb, metasize);
-	}
-
-	if (likely(!xdp_buff_has_frags(xdp)))
-		goto out;
-
-	for (int i = 0; i < nr_frags; i++) {
-		struct skb_shared_info *skinfo = skb_shinfo(skb);
-		skb_frag_t *frag = &sinfo->frags[i];
-		struct page *page;
-		void *addr;
-
-		page = dev_alloc_page();
-		if (!page) {
-			dev_kfree_skb(skb);
-			return NULL;
-		}
-		addr = page_to_virt(page);
-
-		memcpy(addr, skb_frag_page(frag), skb_frag_size(frag));
-
-		__skb_fill_page_desc_noacc(skinfo, skinfo->nr_frags++,
-					   addr, 0, skb_frag_size(frag));
-	}
-
-out:
-	xsk_buff_free(xdp);
-	return skb;
-}
-
 static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring,
 				      struct xdp_buff *xdp_buff,
 				      union i40e_rx_desc *rx_desc,
@@ -372,14 +309,20 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring,
 		 * BIT(I40E_RXD_QW1_ERROR_SHIFT). This is due to that
 		 * SBP is *not* set in PRT_SBPVSI (default not set).
 		 */
-		skb = i40e_construct_skb_zc(rx_ring, xdp_buff);
+		skb = xdp_build_skb_from_zc(xdp_buff);
 		if (!skb) {
+			xsk_buff_free(xdp_buff);
 			rx_ring->rx_stats.alloc_buff_failed++;
 			*rx_packets = 0;
 			*rx_bytes = 0;
 			return;
 		}
 
+		/* xdp_build_skb_from_zc() already ran eth_type_trans();
+		 * restore the header for eth_skb_pad()/process_skb_fields().
+		 */
+		__skb_push(skb, skb->data - skb_mac_header(skb));
+
 		if (eth_skb_pad(skb)) {
 			*rx_packets = 0;
 			*rx_bytes = 0;
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH net-next v3 10/15] ibmveth: Enable multi-queue RX receive path
From: mingming cao @ 2026-07-17  1:27 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, bjking1, haren, ricklind, kuba, edumazet, pabeni,
	linuxppc-dev, maddy, mpe, davemarq
In-Reply-To: <20260714125557.1837451-2-horms@kernel.org>

On 7/14/26 5:55 AM, Simon Horman wrote:

> 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
> ---
> ibmveth: Enable multi-queue RX receive path
>
> This is the first patch that sets multi_queue from H_ILLAN_ATTRIBUTES
> and switches registration, buffer posting, and receive to the MQ
> hcall path. It also raises num_rx_queues and enables per-queue NAPI.
>
>> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
>> index b3b3886c3eedb..863e5c68b42c7 100644
>> --- a/drivers/net/ethernet/ibm/ibmveth.c
>> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> [ ... ]
>
>> @@ -826,15 +841,23 @@ static void ibmveth_replenish_buffer_pool(struct ibmveth_adapter *adapter,
>>    */
>>   static void ibmveth_update_rx_no_buffer(struct ibmveth_adapter *adapter)
>>   {
>> -	__be64 *p = adapter->buffer_list_addr[0] + 4096 - 8;
>> +	int i;
>> +
>> +	for (i = 0; i < adapter->num_rx_queues; i++) {
>> +		__be64 *p = adapter->buffer_list_addr[i] + 4096 - 8;
>> +		u64 drops = be64_to_cpup(p);
>>   
>> -	adapter->rx_no_buffer = be64_to_cpup(p);
>> +		if (i == 0)
>> +			adapter->rx_no_buffer = drops;
>> +	}
>>   }
> [Severity: Medium]
> Will this logic silently discard the hardware drop counts for all queues
> other than queue 0?
>
> Since ethtool -S reports the global adapter->rx_no_buffer statistic, shouldn't
> this loop accumulate the drops across all queues instead of only recording
> them when evaluating the first queue?
>
> [ ... ]
*

Good catch. Per-queue drops are already in rx_qstats

once that patch lands; the global rx_no_buffer still mirrors queue 0

only. I'll sum across queues into rx_no_buffer in v4.

*
>> @@ -842,6 +865,8 @@ static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
>>   				   int queue_index)
>>   {
>> +	struct ibmveth_rx_q *rxq = &adapter->rx_queue[queue_index];
>> +	unsigned long flags;
>>   	int i;
>>   
>>   	if (queue_index >= adapter->num_rx_queues)
>> @@ -842,6 +865,8 @@ static void ibmveth_replenish_task(struct ibmveth_adapter *adapter,
>>   
>>   	adapter->replenish_task_cycles++;
>>   
>> +	spin_lock_irqsave(&rxq->replenish_lock, flags);
>> +
> [Severity: Medium]
> With multi-queue RX enabled, can multiple NAPI threads execute this task
> concurrently for different queues?
>
> If so, does this non-atomic increment of the global replenish_task_cycles
> stat risk data corruption, torn reads on 32-bit platforms, or lost updates?
>
> Similarly, in ibmveth_replenish_buffer_pool():
>
> drivers/net/ethernet/ibm/ibmveth.c:ibmveth_replenish_buffer_pool() {
>      ...
>      buffers_added += filled;
>      adapter->replenish_add_buff_success += filled;
>      remaining -= filled;
>      ...
> }
>
> Even though the caller acquires the per-queue rxq->replenish_lock,
> replenish_add_buff_success is a global adapter statistic. Does this update
> also race with other CPUs holding the locks for their respective queues?
>
> [ ... ]
*Same as on the buffer-submit patch: these are informational counters only.*
*replenish_lock serializes replenishment for a single RX queue, but it 
does not *
*protect the adapter-wide counters across multiple queues.
*
*However, these counters are used only for diagnostics and observability, *
*not for correctness, synchronization, or resource management.While 
stricter *
*accounting could be achieved with atomic operations, these counters are *
*updated in a hot path and the additional synchronization overhead is 
difficult *
*to justify for diagnostic-only statistics, particularly on larger 
systems with *
*many RX queues. If more accurate accounting is desired, a per-queue *
*statistics model would likely be a better follow-up approach, *
>> @@ -2268,34 +2299,40 @@ static void ibmveth_rx_csum_helper(struct sk_buff *skb,
>>   
>>   static int ibmveth_poll(struct napi_struct *napi, int budget)
>>   {
> [ ... ]
>>   		} else {
>>   			struct sk_buff *skb, *new_skb;
>> -			int length = ibmveth_rxq_frame_length(adapter);
>> -			int offset = ibmveth_rxq_frame_offset(adapter);
>> -			int csum_good = ibmveth_rxq_csum_good(adapter);
>> -			int lrg_pkt = ibmveth_rxq_large_packet(adapter);
>> +			int length = ibmveth_rxq_frame_length(adapter, queue_index);
>> +			int offset = ibmveth_rxq_frame_offset(adapter, queue_index);
>> +			int csum_good = ibmveth_rxq_csum_good(adapter, queue_index);
>> +			int lrg_pkt = ibmveth_rxq_large_packet(adapter, queue_index);
> [Severity: High]
> This isn't a bug introduced by this patch, but does this code validate the
> length and offset provided by the hardware descriptor before modifying the
> SKB bounds?
>
> Later in ibmveth_poll(), these values are used directly:
>
> drivers/net/ethernet/ibm/ibmveth.c:ibmveth_poll() {
>      ...
>      skb_reserve(skb, offset);
>      skb_put(skb, length);
>      ...
> }
>
> If the hypervisor provides an untrusted or artificially large length, could
> the call to skb_put() trigger an skb_over_panic() and crash the guest
> kernel?
>
> [ ... ]

*Agreed, this needs guarding.I was already working on a follow-up 
poll-hardening
**patch that adds correlator validation and verifies sufficient 
skb_tailroom() before
calling skb_reserve() and skb_put(). I will keep those validations in v4.
If preferred for bisect safety, I can also move the checks earlier in 
the series rather
than introducing them only in the final hardening patch.*


>
> Also, I noticed a potential memory leak in the error unwinding path of
> ibmveth_open():
>
> drivers/net/ethernet/ibm/ibmveth.c:ibmveth_open() {
>      ...
>      if (rc) {
>          netdev_err(netdev, "failed to set number of rx queues\n");
>          goto out_unregister_queues;
>      }
>      ...
> out_free_buffer_pools:
>      ibmveth_free_buffer_pools(adapter);
> out_unregister_queues:
>      ibmveth_dispose_subordinate_irq_mappings(adapter);
>      ibmveth_free_all_queues(adapter);
> }
>
> [Severity: Medium]
> This is a pre-existing issue, but if netif_set_real_num_rx_queues() fails,
> the code jumps to the out_unregister_queues label.
>
> Does this bypass out_free_buffer_pools, leaving the RX buffer pools unfreed
> and causing a memory leak when cleaning up after a failure?
*

Yes — same open() unwind issue called out on the open/close

refactor patch. Will fix the label order in v4 so buffer pools

are always freed after unregister.

**

Thanks,

Mingming

*

^ permalink raw reply

* [PATCH v2] vhost: reject zero-size IOTLB INVALIDATE
From: Weimin Xiong @ 2026-07-17  1:29 UTC (permalink / raw)
  To: xiongwm2026
  Cc: virtualization, mst, jasowangio, netdev, kvm, xiongweimin,
	Eugenio Perez Martin
In-Reply-To: <20260716030236.124322-1-xiongwm2026@163.com>

From: xiongweimin <xiongweimin@kylinos.cn>

Reject VHOST_IOTLB_INVALIDATE messages with size == 0 to prevent
iova + size - 1 from underflowing to U64_MAX, which would
incorrectly delete the entire IOTLB.

Changes in v2:
- Move the check to vhost_chr_write_iter where similar check for
  VHOST_IOTLB_UPDATE already exists (suggested by Eugenio Perez Martin)
- Add Acked-by from Eugenio Perez Martin

Suggested-by: Eugenio Perez Martin <eperezma@redhat.com>
Acked-by: Eugenio Perez Martin <eperezma@redhat.com>
Signed-off-by: xiongweimin <xiongweimin@kylinos.cn>
---
 drivers/vhost/vhost.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 3c080c454..327e1108c 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1716,7 +1716,8 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
 		goto done;
 	}
 
-	if (msg.type == VHOST_IOTLB_UPDATE && msg.size == 0) {
+	if ((msg.type == VHOST_IOTLB_UPDATE ||
+	     msg.type == VHOST_IOTLB_INVALIDATE) && !msg.size) {
 		ret = -EINVAL;
 		goto done;
 	}
--
2.39.3


^ permalink raw reply related

* Re: [PATCH] vhost: reject zero-size IOTLB INVALIDATE
From: Weimin Xiong @ 2026-07-17  1:29 UTC (permalink / raw)
  To: Eugenio Perez Martin
  Cc: virtualization, mst, jasowangio, netdev, kvm, xiongweimin
In-Reply-To: <CAJaqyWfW9n5o+ojrEjK-m+6cwH_TSxWckjkvEXowZA8r=vDmdQ@mail.gmail.com>

From: xiongweimin <xiongweimin@kylinos.cn>

Hi Eugenio,

Thank you for your review and suggestion!

I've updated the patch to v2, moving the check to vhost_chr_write_iter
as you suggested. The existing check for VHOST_IOTLB_UPDATE is now extended
to also cover VHOST_IOTLB_INVALIDATE.

Thanks for your Acked-by!

Best regards,
Weimin Xiong


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox