* Re: [PATCH net-next v2 3/3] net: mdio: treat PSE EPROBE_DEFER as non-fatal during PHY registration
From: Andrew Lunn @ 2026-04-06 15:21 UTC (permalink / raw)
To: Carlo Szelinsky
Cc: Oleksij Rempel, Kory Maincent, Andrew Lunn, Heiner Kallweit,
Russell King, Jakub Kicinski, David S . Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, netdev, linux-kernel
In-Reply-To: <20260406144324.4007913-1-github@szelinsky.de>
On Mon, Apr 06, 2026 at 04:43:24PM +0200, Carlo Szelinsky wrote:
> Hi Andrew, Oleksij,
>
> Thanks for pushing me in the right direction here.
>
> So if I understand correctly, I should move the PSE control lookup
> from fwnode_mdiobus_register_phy() into phy_probe(). That way the PHY
> just defers like any other device if the PSE controller isn't there
> yet, and the bus scan keeps going for the other PHYs.
Ah, yes. That is the correct fix.
Andrew
^ permalink raw reply
* Re: [PATCH net v2] net: airoha: Add dma_rmb() and READ_ONCE() in airoha_qdma_rx_process()
From: Simon Horman @ 2026-04-06 15:25 UTC (permalink / raw)
To: lorenzo
Cc: 'Simon Horman', andrew+netdev, davem, edumazet, kuba,
pabeni, xuegang.lu, linux-arm-kernel, linux-mediatek, netdev
In-Reply-To: <20260403-airoha_qdma_rx_process-fix-reordering-v2-1-181e6e23d27b@kernel.org>
From: 'Simon Horman' <horms@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net: airoha: Add dma_rmb() and READ_ONCE() in airoha_qdma_rx_process()
This commit adds a missing dma_rmb() to ensure DMA read operations are
completed and uses READ_ONCE() to prevent compiler reordering when accessing
descriptor fields.
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 95ba99b89428e..f1843bc5b9914 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
[ ... ]
> @@ -612,15 +611,17 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
> while (done < budget) {
> struct airoha_queue_entry *e = &q->entry[q->tail];
> struct airoha_qdma_desc *desc = &q->desc[q->tail];
> - u32 hash, reason, msg1 = le32_to_cpu(desc->msg1);
> - struct page *page = virt_to_head_page(e->buf);
> - u32 desc_ctrl = le32_to_cpu(desc->ctrl);
> + u32 hash, reason, msg1, desc_ctrl;
> struct airoha_gdm_port *port;
> int data_len, len, p;
> + struct page *page;
>
> + desc_ctrl = le32_to_cpu(READ_ONCE(desc->ctrl));
> if (!(desc_ctrl & QDMA_DESC_DONE_MASK))
> break;
[ ... ]
> @@ -633,10 +634,12 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
> if (!len || data_len < len)
> goto free_frag;
Could this jump lead to an uninitialized pointer dereference?
The initialization of page was removed from the declarations above, so it
is uninitialized at this point.
Later in the function, the error path looks like this:
free_frag:
if (q->skb) {
dev_kfree_skb(q->skb);
q->skb = NULL;
} else {
page_pool_put_full_page(q->page_pool, page, true);
}
If q->skb is NULL, passing the uninitialized page to
page_pool_put_full_page() might cause a crash.
Also, this isn't a bug introduced by this patch, but does this error path
leak the page associated with the current descriptor?
If we hit an error on a scattered fragment while q->skb is not NULL, the
SKB is freed, but the current page (which hasn't been attached to the SKB
yet) bypasses the else branch and isn't returned to the page pool.
>
> - p = airoha_qdma_get_gdm_port(eth, desc);
> + msg1 = le32_to_cpu(READ_ONCE(desc->msg1));
> + p = airoha_qdma_get_gdm_port(eth, msg1);
> if (p < 0 || !eth->ports[p])
> goto free_frag;
Similarly, taking this jump leaves page uninitialized before reaching the
error handling path.
>
> + page = virt_to_head_page(e->buf);
> port = eth->ports[p];
> if (!q->skb) { /* first buffer */
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] dpll: zl3073x: add ref-sync pair support
From: Jakub Kicinski @ 2026-04-06 15:30 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Arkadiusz Kubalewski, Jiri Pirko, Michal Schmidt,
Petr Oros, Prathosh Satish, Simon Horman, Vadim Fedorenko,
linux-kernel, Conor Dooley, Krzysztof Kozlowski, Rob Herring,
devicetree, Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>
On Sat, 28 Mar 2026 09:06:19 +0100 Ivan Vecera wrote:
> This series adds Reference-Sync pair support to the ZL3073x DPLL driver.
> A Ref-Sync pair consists of a clock reference and a low-frequency sync
> signal (e.g. 1 PPS) where the DPLL locks to the clock reference but
> phase-aligns to the sync reference.
Hi Ivan, FWIW we're waiting for the DT review here.
I suspect some of the DT maintainers are AFK for Easter.
^ permalink raw reply
* Re: [PATCH net-next] selftests: forwarding: lib: rewrite processing of command line arguments
From: Simon Horman @ 2026-04-06 15:31 UTC (permalink / raw)
To: Ioana Ciornei
Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-kernel, petrm, willemb,
linux-kselftest
In-Reply-To: <20260403141912.3825693-1-ioana.ciornei@nxp.com>
On Fri, Apr 03, 2026 at 05:19:12PM +0300, Ioana Ciornei wrote:
> The piece of code which processes the command line arguments and
> populates NETIFS based on them is really unobvious. Rewrite it so that
> the intention is clear and the code is easy to follow.
>
> Suggested-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> tools/testing/selftests/net/forwarding/lib.sh | 22 +++++++++----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index d8cc4c64148d..922cdaf2ceb9 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -466,18 +466,18 @@ if [ "${DRIVER_TEST_CONFORMANT}" = "yes" ]; then
> NETIFS[p2]="$remote_netif"
> TARGETS[$remote_netif]="$REMOTE_TYPE:$REMOTE_ARGS"
> else
> - count=0
AI generated review on Sashiko.dev suggests that initialising count should
be retained to ensure correct behaviour in the (unlikely) case that count
is already set (to a non-zero value) in the environment.
And this seems like a good idea to me.
> + # Prime NETIFS from the command line, but retain if none given.
> + if [[ $# -gt 0 ]]; then
> + unset NETIFS
> + declare -A NETIFS
>
> - while [[ $# -gt 0 ]]; do
> - if [[ "$count" -eq "0" ]]; then
> - unset NETIFS
> - declare -A NETIFS
> - fi
> - count=$((count + 1))
> - NETIFS[p$count]="$1"
> - TARGETS[$1]="local:"
> - shift
> - done
> + while [[ $# -gt 0 ]]; do
> + count=$((count + 1))
> + NETIFS[p$count]="$1"
> + TARGETS[$1]="local:"
> + shift
> + done
> + fi
> fi
>
> ##############################################################################
> --
> 2.25.1
>
^ permalink raw reply
* [PATCH net] xfrm_user: fix info leak in build_mapping()
From: Greg Kroah-Hartman @ 2026-04-06 15:33 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Greg Kroah-Hartman, Steffen Klassert, Herbert Xu,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
struct xfrm_usersa_id has a one-byte padding hole after the proto
field, which ends up never getting set to zero before copying out to
userspace. Fix that up by zeroing out the whole structure before
setting individual variables.
Fixes: 3a2dfbe8acb1 ("xfrm: Notify changes in UDP encapsulation via netlink")
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Note, I think this is correct, as I don't think a new skb has it's
fields pre-zeroed out, or am I totally wrong here?
net/xfrm/xfrm_user.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 8a854fa9567d..1bb8d05561df 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -4165,6 +4165,7 @@ static int build_mapping(struct sk_buff *skb, struct xfrm_state *x,
um = nlmsg_data(nlh);
+ memset(&um->id, 0, sizeof(um->id));
memcpy(&um->id.daddr, &x->id.daddr, sizeof(um->id.daddr));
um->id.spi = x->id.spi;
um->id.family = x->props.family;
--
2.53.0
^ permalink raw reply related
* [PATCH net] xfrm_user: fix info leak in build_report()
From: Greg Kroah-Hartman @ 2026-04-06 15:34 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Greg Kroah-Hartman, stable, Steffen Klassert,
Herbert Xu, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
struct xfrm_user_report is a __u8 proto field followed by a struct
xfrm_selector which means there is three "empty" bytes of padding, but
the padding is never zeroed before copying to userspace. Fix that up by
zeroing the structure before setting individual member variables.
Cc: stable <stable@kernel.org>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
As before, I _think_ this is right, but my code paths might be missing
something where the whole buffer is set to 0 ahead of time somewhere
else?
net/xfrm/xfrm_user.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 6d7b5bb2ec9e..1bb8d05561df 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -4117,6 +4117,7 @@ static int build_report(struct sk_buff *skb, u8 proto,
return -EMSGSIZE;
ur = nlmsg_data(nlh);
+ memset(ur, 0, sizeof(*ur));
ur->proto = proto;
memcpy(&ur->sel, sel, sizeof(ur->sel));
--
2.53.0
^ permalink raw reply related
* Re: [PATCH net] ipv4: icmp: fix null-ptr-deref in icmp_build_probe(): manual merge
From: Jakub Kicinski @ 2026-04-06 15:36 UTC (permalink / raw)
To: Matthieu Baerts
Cc: Yiqi Sun, horms, netdev, davem, dsahern, edumazet, pabeni,
Mark Brown, linux-next
In-Reply-To: <c7df7e27-992e-4973-b6fb-63cdb7b40158@kernel.org>
On Mon, 6 Apr 2026 12:48:28 +0200 Matthieu Baerts wrote:
> - dev = ipv6_stub->ipv6_dev_find(net, &iio->ident.addr.ip_addr.ipv6_addr, dev);
> + dev = ipv6_dev_find(net, &iio->ident.addr.ip_addr.ipv6_addr, dev);
> + /*
> + * If IPv6 identifier lookup is unavailable, silently
> + * discard the request instead of misreporting NO_IF.
> + */
> + if (IS_ERR(dev))
> + return false;
> +
> dev_hold(dev);
> break;
See my reply, AFAIU we can just delete this new check in net-next.
^ permalink raw reply
* Re: [PATCH v2] stmmac: cleanup dead dependencies on STMMAC_PLATFORM and STMMAC_ETH in Kconfig
From: Jakub Kicinski @ 2026-04-06 15:39 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Julian Braha, davem, peppe.cavallaro, alexandre.torgue,
mcoquelin.stm32, linux, netdev, linux-arm-kernel, linux-kernel,
Russell King (Oracle)
In-Reply-To: <CAMuHMdUfzVSQpadJYpEqPJ_UOBAgswnGzD_bp_U3U6jt2dy0dg@mail.gmail.com>
On Mon, 6 Apr 2026 10:23:46 +0200 Geert Uytterhoeven wrote:
> > config STMMAC_PLATFORM
> > tristate "STMMAC Platform bus support"
> > - depends on STMMAC_ETH
> > select MFD_SYSCON
> > default y
>
> This now lets us have STMMAC_PLATFORM=y and STMMAC_ETH=m.
> Does that actually link?
Hm. Sashiko didn't complain when patch was posted.
Typical LLM indeterminism?
^ permalink raw reply
* Re: [PATCH net v3 4/7] net/sched: netem: restructure dequeue to avoid re-entrancy with child qdisc
From: Jakub Kicinski @ 2026-04-06 15:41 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Simon Horman, netdev, Jamal Hadi Salim, Jiri Pirko,
David S. Miller, Eric Dumazet, Paolo Abeni, open list
In-Reply-To: <20260404094946.GU113102@horms.kernel.org>
On Sat, 4 Apr 2026 10:49:46 +0100 Simon Horman wrote:
> On Thu, Apr 02, 2026 at 01:19:32PM -0700, Stephen Hemminger wrote:
> > netem_dequeue() enqueues packets into its child qdisc while being
> > called from the parent's dequeue path. This causes two problems:
> >
> > - HFSC tracks class active/inactive state on qlen transitions.
> > A child enqueue during dequeue causes double-insertion into
> > the eltree (CVE-2025-37890, CVE-2025-38001).
> >
> > - Non-work-conserving children like TBF may refuse to dequeue
> > packets just enqueued, causing netem to return NULL despite
> > having backlog. Parents like DRR then incorrectly deactivate
> > the class.
> >
> > Split the dequeue into helpers:
> >
> > netem_pull_tfifo() - remove head packet from tfifo
> > netem_slot_account() - update slot pacing counters
> > netem_dequeue_child() - batch-transfer ready packets to the
> > child, then dequeue from the child
> > netem_dequeue_direct()- dequeue from tfifo when no child
> >
> > When a child qdisc is present, all time-ready packets are moved
> > into the child before calling its dequeue. This separates the
> > enqueue and dequeue phases so the parent sees consistent qlen
> > transitions.
> >
> > Fixes: 50612537e9ab ("netem: fix classful handling")
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > net/sched/sch_netem.c | 201 +++++++++++++++++++++++++++---------------
> > 1 file changed, 128 insertions(+), 73 deletions(-)
>
> Hi Stephen,
>
> As a fix this is a large and complex patch.
> Could it be split up somehow to aid review?
+1, FWIW it's perfectly fine to have refactoring patch in a net series
(without a Fixes tag) if it makes the fix a lot easier to review.
^ permalink raw reply
* Re: [PATCH net-next V2 4/5] net/mlx5e: XDP, Use a single linear page per rq
From: Jakub Kicinski @ 2026-04-06 15:43 UTC (permalink / raw)
To: Dragos Tatulea
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Saeed Mahameed, Leon Romanovsky, Mark Bloch,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, Cosmin Ratiu, Simon Horman,
Jacob Keller, Lama Kayal, Michal Swiatkowski, Carolina Jubran,
Nathan Chancellor, Daniel Zahka, Rahul Rameshbabu, Raed Salem,
netdev, linux-rdma, linux-kernel, bpf, Gal Pressman
In-Reply-To: <adH5yAsPJ8rNgT0k@x13>
On Sun, 5 Apr 2026 08:08:06 +0200 Dragos Tatulea wrote:
> sashiko says:
Thanks a lot for reviewing the review! It takes a lot of maintainer time
^ permalink raw reply
* Re: [PATCH net v2 1/1] net: l3mdev: Ignore non-L3 uppers in l3mdev_fib_table_rcu
From: Ido Schimmel @ 2026-04-06 15:48 UTC (permalink / raw)
To: Ao Zhou
Cc: netdev, David Ahern, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Ido Schimmel,
Jiri Pirko, Yifan Wu, Juefei Pu, Yuan Tan, Xin Liu, Haoze Xie
In-Reply-To: <429dd4a81d4ca5624ab9f6d7b53c5fe08552c734.1775443332.git.royenheart@gmail.com>
On Mon, Apr 06, 2026 at 09:28:16PM +0800, Ao Zhou wrote:
> From: Haoze Xie <royenheart@gmail.com>
>
> l3mdev_fib_table_rcu() assumes that any upper device observed for
> an IFF_L3MDEV_SLAVE device is an L3 master and dereferences
> master->l3mdev_ops unconditionally.
>
> VRF slave setup sets IFF_L3MDEV_SLAVE before the upper link is fully
> switched, so readers can transiently observe a non-L3 upper such as a
> bridge and follow a NULL l3mdev_ops pointer. Require the current upper
> to still be an L3 master before consulting its FIB table.
>
> Fixes: fdeea7be88b1 ("net: vrf: Set slave's private flag before linking")
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Reviewed-by: David Ahern <dsahern@kernel.org>
> Signed-off-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Ao Zhou <n05ec@lzu.edu.cn>
> ---
> changes in v2:
> - point Fixes to the VRF slave ordering change identified by David Ahern
> - add David Ahern's Reviewed-by trailer
>
> net/l3mdev/l3mdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/l3mdev/l3mdev.c b/net/l3mdev/l3mdev.c
> index 5432a5f2dfc8..b8a3030cb2c4 100644
> --- a/net/l3mdev/l3mdev.c
> +++ b/net/l3mdev/l3mdev.c
> @@ -177,7 +177,7 @@ u32 l3mdev_fib_table_rcu(const struct net_device *dev)
> const struct net_device *master;
>
> master = netdev_master_upper_dev_get_rcu(_dev);
> - if (master &&
> + if (master && netif_is_l3_master(master) &&
> master->l3mdev_ops->l3mdev_fib_table)
Don't we have the same problem in l3mdev_l3_rcv() and l3mdev_l3_out()?
If so, please check if I missed more places and include them in v3.
And I think that the part that I was missing earlier is that we don't
have RCU synchronization in the unslaving path, so an RCU reader can
either see the original master, NULL or a new master (e.g., bridge
instead of the original VRF master).
> tb_id = master->l3mdev_ops->l3mdev_fib_table(master);
> }
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH net] xfrm_user: fix info leak in build_mapping()
From: Jakub Kicinski @ 2026-04-06 15:54 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: netdev, linux-kernel, Steffen Klassert, Herbert Xu,
David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman
In-Reply-To: <2026040602-shack-tamale-d8c3@gregkh>
On Mon, 6 Apr 2026 17:33:03 +0200 Greg Kroah-Hartman wrote:
> struct xfrm_usersa_id has a one-byte padding hole after the proto
> field, which ends up never getting set to zero before copying out to
> userspace. Fix that up by zeroing out the whole structure before
> setting individual variables.
>
> Fixes: 3a2dfbe8acb1 ("xfrm: Notify changes in UDP encapsulation via netlink")
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Assisted-by: gregkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> Note, I think this is correct, as I don't think a new skb has it's
> fields pre-zeroed out, or am I totally wrong here?
You're right, skb owner is responsible for clearing after put.
Tho, Netlink is not as perf critical as real networking, I wish
we at least had a helper which reserves the space and clears it :/
This is not the first or the second time we hit this sort of a bug.
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 8a854fa9567d..1bb8d05561df 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -4165,6 +4165,7 @@ static int build_mapping(struct sk_buff *skb, struct xfrm_state *x,
>
> um = nlmsg_data(nlh);
>
> + memset(&um->id, 0, sizeof(um->id));
> memcpy(&um->id.daddr, &x->id.daddr, sizeof(um->id.daddr));
> um->id.spi = x->id.spi;
> um->id.family = x->props.family;
^ permalink raw reply
* Re: [PATCH net] xfrm_user: fix info leak in build_mapping()
From: Jakub Kicinski @ 2026-04-06 15:58 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: netdev, linux-kernel, Steffen Klassert, Herbert Xu,
David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman
In-Reply-To: <20260406085449.266466ae@kernel.org>
On Mon, 6 Apr 2026 08:54:49 -0700 Jakub Kicinski wrote:
> > Note, I think this is correct, as I don't think a new skb has it's
> > fields pre-zeroed out, or am I totally wrong here?
>
> You're right, skb owner is responsible for clearing after put.
> Tho, Netlink is not as perf critical as real networking, I wish
> we at least had a helper which reserves the space and clears it :/
> This is not the first or the second time we hit this sort of a bug.
We could make nlmsg_append() do that. Mostly because I don't have
a better idea for a name and nlmsg_append is only used once ;)
^ permalink raw reply
* Re: [PATCH net-next v2] selftests/net: convert so_txtime to drv-net
From: Jakub Kicinski @ 2026-04-06 16:02 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, edumazet, pabeni, horms, Willem de Bruijn
In-Reply-To: <willemdebruijn.kernel.1622322e50a5d@gmail.com>
On Sat, 04 Apr 2026 22:20:15 -0400 Willem de Bruijn wrote:
> > +YNL_GEN_FILES := \
> > + psp_responder \
> > + so_txtime
>
> This should just go under TEST_GEN_FILES (sashiko)
>
> I don't quite understand the check_selftest check_new_files_makefile
> failure.
Glancing at the code you were missing the
# end of YNL_GEN_FILES
trailer.
Those checkers are equally annoying and easy to run locally, they
take a path to the modified file and say "good" / "bad".
^ permalink raw reply
* Re: [PATCH bpf v2 1/2] bpf: Fix same-register dst/src OOB read and pointer leak in sock_ops
From: sun jian @ 2026-04-06 16:06 UTC (permalink / raw)
To: Jiayuan Chen
Cc: bpf, Quan Sun, Yinhao Hu, Kaiyan Mei, Dongliang Mu,
Emil Tsalapatis, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Jiri Olsa,
John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
linux-kernel, netdev, linux-kselftest
In-Reply-To: <20260406031330.187630-1-jiayuan.chen@linux.dev>
On Mon, Apr 6, 2026 at 11:14 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> When a BPF sock_ops program accesses ctx fields with dst_reg == src_reg,
> the SOCK_OPS_GET_SK() and SOCK_OPS_GET_FIELD() macros fail to zero the
> destination register in the !fullsock / !locked_tcp_sock path.
>
> Both macros borrow a temporary register to check is_fullsock /
> is_locked_tcp_sock when dst_reg == src_reg, because dst_reg holds the
> ctx pointer. When the check is false (e.g., TCP_NEW_SYN_RECV state with
> a request_sock), dst_reg should be zeroed but is not, leaving the stale
> ctx pointer:
>
> - SOCK_OPS_GET_SK: dst_reg retains the ctx pointer, passes NULL checks
> as PTR_TO_SOCKET_OR_NULL, and can be used as a bogus socket pointer,
> leading to stack-out-of-bounds access in helpers like
> bpf_skc_to_tcp6_sock().
>
> - SOCK_OPS_GET_FIELD: dst_reg retains the ctx pointer which the
> verifier believes is a SCALAR_VALUE, leaking a kernel pointer.
>
> Fix both macros by:
> - Changing JMP_A(1) to JMP_A(2) in the fullsock path to skip the
> added instruction.
> - Adding BPF_MOV64_IMM(si->dst_reg, 0) after the temp register
> restore in the !fullsock path, placed after the restore because
> dst_reg == src_reg means we need src_reg intact to read ctx->temp.
>
> Fixes: 84f44df664e9 ("bpf: sock_ops sk access may stomp registers when dst_reg = src_reg")
> Reported-by: Quan Sun <2022090917019@std.uestc.edu.cn>
> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
> Closes: https://lore.kernel.org/bpf/6fe1243e-149b-4d3b-99c7-fcc9e2f75787@std.uestc.edu.cn/T/#u
> Suggested-by: Emil Tsalapatis <emil@etsalapatis.com>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
> ---
> net/core/filter.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 78b548158fb05..53ce06ed4a88e 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -10581,10 +10581,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
> si->dst_reg, si->dst_reg, \
> offsetof(OBJ, OBJ_FIELD)); \
> if (si->dst_reg == si->src_reg) { \
> - *insn++ = BPF_JMP_A(1); \
> + *insn++ = BPF_JMP_A(2); \
> *insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg, \
> offsetof(struct bpf_sock_ops_kern, \
> temp)); \
> + *insn++ = BPF_MOV64_IMM(si->dst_reg, 0); \
> } \
> } while (0)
>
> @@ -10618,10 +10619,11 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
> si->dst_reg, si->src_reg, \
> offsetof(struct bpf_sock_ops_kern, sk));\
> if (si->dst_reg == si->src_reg) { \
> - *insn++ = BPF_JMP_A(1); \
> + *insn++ = BPF_JMP_A(2); \
> *insn++ = BPF_LDX_MEM(BPF_DW, reg, si->src_reg, \
> offsetof(struct bpf_sock_ops_kern, \
> temp)); \
> + *insn++ = BPF_MOV64_IMM(si->dst_reg, 0); \
> } \
> } while (0)
>
> --
> 2.43.0
>
>
Reviewed-by Sun Jian <sun.jian.kdev@gmail.com>
^ permalink raw reply
* Re: [PATCH net] xfrm_user: fix info leak in build_mapping()
From: Greg Kroah-Hartman @ 2026-04-06 16:08 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, linux-kernel, Steffen Klassert, Herbert Xu,
David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman
In-Reply-To: <20260406085859.2e9843ea@kernel.org>
On Mon, Apr 06, 2026 at 08:58:59AM -0700, Jakub Kicinski wrote:
> On Mon, 6 Apr 2026 08:54:49 -0700 Jakub Kicinski wrote:
> > > Note, I think this is correct, as I don't think a new skb has it's
> > > fields pre-zeroed out, or am I totally wrong here?
> >
> > You're right, skb owner is responsible for clearing after put.
> > Tho, Netlink is not as perf critical as real networking, I wish
> > we at least had a helper which reserves the space and clears it :/
> > This is not the first or the second time we hit this sort of a bug.
>
> We could make nlmsg_append() do that. Mostly because I don't have
> a better idea for a name and nlmsg_append is only used once ;)
As shown in my other patch:
https://lore.kernel.org/r/2026040621-poison-gristle-aaa3@gregkh
we need this in at least 2 places, don't know if it's worth doing it for
all messages?
I guess nlmsg_append() would work? It tries to do some zeroing out for
alignment for some reason...
Want me to do that? I don't have a way to test any of this, I just
found it using some static code analysis tools that looked at holes in
structures.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net] ipv4: icmp: fix null-ptr-deref in icmp_build_probe(): manual merge
From: Matthieu Baerts @ 2026-04-06 16:10 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Yiqi Sun, horms, netdev, davem, dsahern, edumazet, pabeni,
Mark Brown, linux-next
In-Reply-To: <20260406083615.32f44e4f@kernel.org>
Hi Jakub,
Thank you for your reply!
On 06/04/2026 17:36, Jakub Kicinski wrote:
> On Mon, 6 Apr 2026 12:48:28 +0200 Matthieu Baerts wrote:
>> - dev = ipv6_stub->ipv6_dev_find(net, &iio->ident.addr.ip_addr.ipv6_addr, dev);
>> + dev = ipv6_dev_find(net, &iio->ident.addr.ip_addr.ipv6_addr, dev);
>> + /*
>> + * If IPv6 identifier lookup is unavailable, silently
>> + * discard the request instead of misreporting NO_IF.
>> + */
>> + if (IS_ERR(dev))
>> + return false;
>> +
>> dev_hold(dev);
>> break;
>
> See my reply, AFAIU we can just delete this new check in net-next.
Good idea.
Should this not be done in an explicit patch, rather than "hidden"
during the merge?
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply
* Re: [PATCH net-next V4 10/12] devlink: Add resource scope filtering to resource dump
From: Or Har-Toov @ 2026-04-06 16:18 UTC (permalink / raw)
To: Jakub Kicinski, Tariq Toukan
Cc: Eric Dumazet, Paolo Abeni, Andrew Lunn, David S. Miller,
Simon Horman, Donald Hunter, Jiri Pirko, Jonathan Corbet,
Shuah Khan, Saeed Mahameed, Leon Romanovsky, Mark Bloch,
Shuah Khan, Chuck Lever, Matthieu Baerts (NGI0), Carolina Jubran,
Moshe Shemesh, Dragos Tatulea, Shahar Shitrit, Daniel Zahka,
Jacob Keller, Cosmin Ratiu, Parav Pandit, Shay Drori,
Adithya Jayachandran, Kees Cook, Daniel Jurgens, netdev,
linux-kernel, linux-doc, linux-rdma, linux-kselftest,
Gal Pressman
In-Reply-To: <20260402190219.61ea7da1@kernel.org>
On 03/04/2026 5:02, Jakub Kicinski wrote:
>
> On Wed, 1 Apr 2026 21:49:45 +0300 Tariq Toukan wrote:
>> @@ -873,6 +881,16 @@ attribute-sets:
>> doc: Unique devlink instance index.
>> checks:
>> max: u32-max
>> + -
>> + name: resource-scope-mask
>> + type: bitfield32
>
> no need for a bitfield here, this is a simpler selector
> bitfield is for cases when we need to update some persistent
> state, in that case we want to indicate which bits we intend
> to update:
>
> cfg = (cfg & ~bf.mask) | bf.val
>
> scope is a straight attribute, there's no updating of anything.
>
> u32 or unit would do
ack, will use u32 for resource scope mask
>
>> + enum: resource-scope
>> + enum-as-flags: true
>> + doc: |
>> + Bitmask selecting which resource classes to include in a
>> + resource-dump response. Bit 0 (dev) selects device-level
>> + resources; bit 1 (port) selects port-level resources.
>> + When absent all classes are returned.
>> -
>> name: dl-dev-stats
>> subset-of: devlink
>> @@ -1775,7 +1793,11 @@ operations:
>> - resource-list
>> dump:
>> request:
>> - attributes: *dev-id-attrs
>> + attributes:
>> + - bus-name
>> + - dev-name
>> + - index
>> + - resource-scope-mask
>> reply: *resource-dump-reply
>>
>> -
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 7de2d8cc862f..e0a0b523ce5c 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -645,6 +645,7 @@ enum devlink_attr {
>> DEVLINK_ATTR_PARAM_RESET_DEFAULT, /* flag */
>>
>> DEVLINK_ATTR_INDEX, /* uint */
>> + DEVLINK_ATTR_RESOURCE_SCOPE_MASK, /* bitfield32 */
>>
>> /* Add new attributes above here, update the spec in
>> * Documentation/netlink/specs/devlink.yaml and re-generate
>> @@ -704,6 +705,22 @@ enum devlink_resource_unit {
>> DEVLINK_RESOURCE_UNIT_ENTRY,
>> };
>>
>> +enum devlink_resource_scope {
>> + DEVLINK_RESOURCE_SCOPE_DEV_BIT,
>> + DEVLINK_RESOURCE_SCOPE_PORT_BIT,
>> +
>> + __DEVLINK_RESOURCE_SCOPE_MAX_BIT,
>> + DEVLINK_RESOURCE_SCOPE_MAX_BIT =
>
> do we need this? it's not an attr enum all we care about here is
> the mask, really so just a trailing value which is max real value + 1
> is enough for all users?
>
ok, can manage without max at all, can manage also without
DEVLINK_RESOURCE_SCOPE_VALID_MASK
>> + __DEVLINK_RESOURCE_SCOPE_MAX_BIT - 1
>> +};
>> +
>> +#define DEVLINK_RESOURCE_SCOPE_DEV \
>> + _BITUL(DEVLINK_RESOURCE_SCOPE_DEV_BIT)
>> +#define DEVLINK_RESOURCE_SCOPE_PORT \
>> + _BITUL(DEVLINK_RESOURCE_SCOPE_PORT_BIT)
>> +#define DEVLINK_RESOURCE_SCOPE_VALID_MASK \
>> + (_BITUL(__DEVLINK_RESOURCE_SCOPE_MAX_BIT) - 1)
>> +
>> enum devlink_port_fn_attr_cap {
>> DEVLINK_PORT_FN_ATTR_CAP_ROCE_BIT,
>> DEVLINK_PORT_FN_ATTR_CAP_MIGRATABLE_BIT,
>
>> +static u32 devlink_resource_scope_get(struct nlattr **attrs, int *flags)
>> +{
>> + struct nla_bitfield32 scope;
>> + u32 value;
>> +
>> + if (!attrs || !attrs[DEVLINK_ATTR_RESOURCE_SCOPE_MASK])
>> + return DEVLINK_RESOURCE_SCOPE_VALID_MASK;
>> +
>> + scope = nla_get_bitfield32(attrs[DEVLINK_ATTR_RESOURCE_SCOPE_MASK]);
>> + value = scope.value & scope.selector;
>> + if (value != DEVLINK_RESOURCE_SCOPE_VALID_MASK)
>> + *flags |= NLM_F_DUMP_FILTERED;
>> +
>> + return value;
>> +}
>> +
>> static int
>> devlink_resource_dump_fill_one(struct sk_buff *skb, struct devlink *devlink,
>> struct devlink_port *devlink_port,
>> @@ -400,16 +416,27 @@ devlink_nl_resource_dump_one(struct sk_buff *skb, struct devlink *devlink,
>> struct devlink_nl_dump_state *state = devlink_dump_state(cb);
>> struct devlink_port *devlink_port;
>> unsigned long port_idx;
>> + u32 scope;
>> int err;
>>
>> - if (!state->port_number) {
>> + scope = devlink_resource_scope_get(genl_info_dump(cb)->attrs, &flags);
>> + if (!scope) {
>> + NL_SET_ERR_MSG_ATTR(genl_info_dump(cb)->extack,
>> + genl_info_dump(cb)->attrs[DEVLINK_ATTR_RESOURCE_SCOPE_MASK],
>
> we have genl_info_dump(cb) 3 times here, let's save the pointer
> on the stack to make the lines shorter.
>
ack
>> + "empty resource scope selection");
>> + return -EINVAL;
>> + }
>> + if (!state->port_number && (scope & DEVLINK_RESOURCE_SCOPE_DEV)) {
>> err = devlink_resource_dump_fill_one(skb, devlink, NULL,
>> - cb, flags, &state->idx);
>> + cb, flags,
>> + &state->idx);
>> if (err)
>> return err;
>> state->idx = 0;
>> }
>>
>> + if (!(scope & DEVLINK_RESOURCE_SCOPE_PORT))
>> + goto out;
>> xa_for_each_start(&devlink->ports, port_idx, devlink_port,
>> state->port_number ? state->port_number - 1 : 0) {
>> err = devlink_resource_dump_fill_one(skb, devlink, devlink_port,
>> @@ -420,6 +447,7 @@ devlink_nl_resource_dump_one(struct sk_buff *skb, struct devlink *devlink,
>> }
>> state->idx = 0;
>> }
>> +out:
>> state->port_number = 0;
>> return 0;
>> }
>
^ permalink raw reply
* Re: [net-next v2 2/3] selftests/net: Test PACKET_STATISTICS drops
From: Joe Damato @ 2026-04-06 16:29 UTC (permalink / raw)
To: Willem de Bruijn
Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Shuah Khan, andrew+netdev,
linux-kernel, willemb, linux-kselftest
In-Reply-To: <willemdebruijn.kernel.aa6e2a3e845d@gmail.com>
On Sat, Apr 04, 2026 at 11:08:15AM -0400, Willem de Bruijn wrote:
> Joe Damato wrote:
[...]
> > @@ -396,6 +424,11 @@ static void run_test(void)
> >
> > total_len = do_tx();
> >
> > + if (cfg_drop) {
> > + check_packet_stats(fds);
> > + goto out;
> > + }
>
> Sashiko pointed out
In the future, please link to the AI report.
> that there may be a race here between rx
> processing in the softirq and check_packet_stats. Seems plausible.
Can you elaborate why this is true?
I could be wrong, but I scanned the code and __local_bh_enable_ip seems to run
softirq synchronously in this case, so I think the code is correct.
^ permalink raw reply
* Re: [PATCH net-next V2 4/5] net/mlx5e: XDP, Use a single linear page per rq
From: Mark Bloch @ 2026-04-06 16:31 UTC (permalink / raw)
To: Jakub Kicinski, Dragos Tatulea
Cc: Tariq Toukan, Eric Dumazet, Paolo Abeni, Andrew Lunn,
David S. Miller, Saeed Mahameed, Leon Romanovsky,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, Cosmin Ratiu, Simon Horman,
Jacob Keller, Lama Kayal, Michal Swiatkowski, Carolina Jubran,
Nathan Chancellor, Daniel Zahka, Rahul Rameshbabu, Raed Salem,
netdev, linux-rdma, linux-kernel, bpf, Gal Pressman
In-Reply-To: <20260406084344.5d315f01@kernel.org>
On 06/04/2026 18:43, Jakub Kicinski wrote:
> On Sun, 5 Apr 2026 08:08:06 +0200 Dragos Tatulea wrote:
>> sashiko says:
>
> Thanks a lot for reviewing the review! It takes a lot of maintainer time
Just to add some context: we started running Sashiko internally,
so hopefully trivial issues won’t be missed. I don’t know if
you remember our on-list discussion from a few weeks ago, following that
discussion right now we have three different internal AI tools reviewing each
commit.
At the moment this is still manageable, and I think developers should
look over all comments from all tools. In our case that currently
means three review outputs per commit. It would also be useful to have
some official guidance on what authors are recommended to run before
posting, so obvious issues can be caught earlier and less reviewer/maintainer
time is spent on them.
For example:
“Before posting, authors could run a recommended baseline of review tools,
where available, to catch obvious issues early. During review, tools such
as review-prompts and Sashiko may be used to assist the reviewer.”
Mark
^ permalink raw reply
* Re: [net-next v2 3/3] selftests/net: Test PACKET_AUXDATA
From: Joe Damato @ 2026-04-06 16:36 UTC (permalink / raw)
To: Willem de Bruijn
Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Shuah Khan, andrew+netdev,
linux-kernel, willemb, linux-kselftest
In-Reply-To: <willemdebruijn.kernel.3ad8f36da2a1f@gmail.com>
On Sat, Apr 04, 2026 at 11:10:13AM -0400, Willem de Bruijn wrote:
> Joe Damato wrote:
> > Extend the packet socket selftest, adding a recvmsg path, to test
> > PACKET_AUXDATA. Check basic attributes of tpacket_auxdata.
> >
> > Signed-off-by: Joe Damato <joe@dama.to>
>
> > @@ -322,8 +376,11 @@ static void parse_opts(int argc, char **argv)
> > {
> > int c;
> >
> > - while ((c = getopt(argc, argv, "bcCdDgl:qt:vV")) != -1) {
> > + while ((c = getopt(argc, argv, "abcCdDgl:qt:vV")) != -1) {
> > switch (c) {
> > + case 'a':
> > + cfg_aux_data = true;
> > + break;
>
> These are intended as independent test cases: test_auxdata,
> test_drops. By using flags the flags can be combined. Are all
> combinations of flags expected to work? Else maybe we should instead
> have a -T "test_name", or so, to keep them mutually exclusive and
> easy to reason about.
>
> Or just a test that two flags are not set at the same time.
Sure, I can add a two line at the bottom with the existing flags checks. I
don't think adding a -T "test_name" is worth the effort, though.
^ permalink raw reply
* Re: [PATCH net-next 0/3] net: phy: add support for disabling autonomous EEE
From: Florian Fainelli @ 2026-04-06 16:43 UTC (permalink / raw)
To: Andrew Lunn, Nicolai Buchwitz
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Broadcom internal kernel review list,
netdev, linux-kernel
In-Reply-To: <5298959c-12de-4992-a896-3d1371cba111@lunn.ch>
On 4/6/26 06:56, Andrew Lunn wrote:
>> "Autonomous EEE" works for me (obviously). It kinda describes what
>> happens (the PHY acts autonomously) without being tied to a specific
>> vendor's marketing name.
>>
>> That said, I feel Russell's argument that it doesn't type well. But
>> even after some walks in the woods I wasn't able to come up with a
>> better name yet.
>
> Yes, naming is hard.
>
>> I don't think there's a meaningful use case for letting users choose
>> between the two. Or is there?
>
> This has come up before, in other context. PTP is one example, where
> both the MAC and the PHY can implement it. Ideally, you want it to
> happen at the PHY, it gives higher accuracy. But there are systems
> where the PHY implementation is somewhat broken, and the MAC PTP gives
> better results. So defaulting to PHY PTP is not always the best
> option.
>
> I would expect IEEE 802.3 EEE, with both the MAC and PHY cooperating
> is better than just PHY level EEE, the system as a whole has a better
> picture of what is going on, so using 802.3 EEE should be the
> preference/default. But are we going to run into oddball systems where
> autonomous EEE is actually better than using a somewhat broke MAC?
That is almost a guarantee given, there will be a broken MAC, and there
will be a need for enabling EEE at the PHY level because the power
savings can be significant. Until that happens however, I would agree
with using the global "--set-eee" without differentiating, and if we
need to disable MAC-level EEE and switch to PHY-level EEE then we cross
that bridge when necessary.
--
Florian
^ permalink raw reply
* Re: [PATCH net] ipv4: icmp: fix null-ptr-deref in icmp_build_probe(): manual merge
From: Jakub Kicinski @ 2026-04-06 16:46 UTC (permalink / raw)
To: Matthieu Baerts
Cc: Yiqi Sun, horms, netdev, davem, dsahern, edumazet, pabeni,
Mark Brown, linux-next
In-Reply-To: <80348cc5-195d-4acf-9d20-2ddc0da92ef5@kernel.org>
On Mon, 6 Apr 2026 18:10:14 +0200 Matthieu Baerts wrote:
> On 06/04/2026 17:36, Jakub Kicinski wrote:
> > On Mon, 6 Apr 2026 12:48:28 +0200 Matthieu Baerts wrote:
> >> - dev = ipv6_stub->ipv6_dev_find(net, &iio->ident.addr.ip_addr.ipv6_addr, dev);
> >> + dev = ipv6_dev_find(net, &iio->ident.addr.ip_addr.ipv6_addr, dev);
> >> + /*
> >> + * If IPv6 identifier lookup is unavailable, silently
> >> + * discard the request instead of misreporting NO_IF.
> >> + */
> >> + if (IS_ERR(dev))
> >> + return false;
> >> +
> >> dev_hold(dev);
> >> break;
> >
> > See my reply, AFAIU we can just delete this new check in net-next.
>
> Good idea.
>
> Should this not be done in an explicit patch, rather than "hidden"
> during the merge?
Dunno, it's not a huge change, feels like a good fit for a merge.
^ permalink raw reply
* Re: [PATCH v2] stmmac: cleanup dead dependencies on STMMAC_PLATFORM and STMMAC_ETH in Kconfig
From: Russell King (Oracle) @ 2026-04-06 16:46 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Julian Braha, davem, peppe.cavallaro, alexandre.torgue,
mcoquelin.stm32, kuba, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <CAMuHMdUfzVSQpadJYpEqPJ_UOBAgswnGzD_bp_U3U6jt2dy0dg@mail.gmail.com>
On Mon, Apr 06, 2026 at 10:23:46AM +0200, Geert Uytterhoeven wrote:
> Hi Julian,
>
> On Thu, 2 Apr 2026 at 17:07, Julian Braha <julianbraha@gmail.com> wrote:
> > There are already 'if STMMAC_ETH' and 'STMMAC_PLATFORM'
> > conditions wrapping these config options, making the
> > 'depends on' statements duplicate dependencies (dead code).
> >
> > I propose leaving the outer 'if STMMAC_PLATFORM...endif' and
> > 'if STMMAC_ETH...endif' conditions, and removing the
> > individual 'depends on' statements.
> >
> > This dead code was found by kconfirm, a static analysis tool for Kconfig.
> >
> > Signed-off-by: Julian Braha <julianbraha@gmail.com>
> > Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> > v2: add back default STMMAC_PLATFORM for DWMAC_GENERIC
> > Link to v1: https://lore.kernel.org/all/20260331125817.117091-1-julianbraha@gmail.com/
>
> Thanks for your patch, which is now commit e2f152c822cf5d37 ("stmmac:
> cleanup dead dependencies on STMMAC_PLATFORM and STMMAC_ETH in
> Kconfig") in net-next.
>
> > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > @@ -20,7 +20,6 @@ if STMMAC_ETH
> > config STMMAC_SELFTESTS
> > bool "Support for STMMAC Selftests"
> > depends on INET
> > - depends on STMMAC_ETH
> > default n
> > help
> > This adds support for STMMAC Selftests using ethtool. Enable this
> > @@ -29,7 +28,6 @@ config STMMAC_SELFTESTS
> >
> > config STMMAC_PLATFORM
> > tristate "STMMAC Platform bus support"
> > - depends on STMMAC_ETH
> > select MFD_SYSCON
> > default y
>
> This now lets us have STMMAC_PLATFORM=y and STMMAC_ETH=m.
> Does that actually link?
Really?
Let's try forcing it:
CONFIG_STMMAC_ETH=m
CONFIG_STMMAC_SELFTESTS=y
CONFIG_STMMAC_PLATFORM=y
and make oldconfig...
STMicroelectronics Multi-Gigabit Ethernet driver (STMMAC_ETH) [M/n/y/?] m
Support for STMMAC Selftests (STMMAC_SELFTESTS) [Y/n/?] y
STMMAC Platform bus support (STMMAC_PLATFORM) [M/n/?] m
CONFIG_STMMAC_PLATFORM is forced to 'm'. 'y' is not permitted.
I think you're mistaken.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH bpf v2 2/2] selftests/bpf: Add tests for sock_ops ctx access with same src/dst register
From: sun jian @ 2026-04-06 16:47 UTC (permalink / raw)
To: Jiayuan Chen
Cc: bpf, Martin KaFai Lau, Daniel Borkmann, John Fastabend,
Stanislav Fomichev, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Shuah Khan,
linux-kernel, netdev, linux-kselftest
In-Reply-To: <20260406031330.187630-2-jiayuan.chen@linux.dev>
On Mon, Apr 6, 2026 at 11:15 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> Add selftests to verify SOCK_OPS_GET_SK() and SOCK_OPS_GET_FIELD()
> correctly return NULL/zero when dst_reg == src_reg and is_fullsock == 0.
>
> Three subtests are included:
> - get_sk: ctx->sk with same src/dst register (SOCK_OPS_GET_SK)
> - get_field: ctx->snd_cwnd with same src/dst register (SOCK_OPS_GET_FIELD)
> - get_sk_diff_reg: ctx->sk with different src/dst register (baseline)
>
> Each BPF program uses inline asm (__naked) to force specific register
> allocation, reads is_fullsock first, then loads the field using the same
> (or different) register. The test triggers TCP_NEW_SYN_RECV via a TCP
> handshake and checks that the result is NULL/zero when is_fullsock == 0.
>
> Test:
> ./test_progs -a sock_ops_get_sk -v
> test_sock_ops_get_sk:PASS:join_cgroup 0 nsec
> test_sock_ops_get_sk:PASS:skel_open_load 0 nsec
> run_sock_ops_test:PASS:prog_attach 0 nsec
> run_sock_ops_test:PASS:start_server 0 nsec
> run_sock_ops_test:PASS:connect_to_fd 0 nsec
> test_sock_ops_get_sk:PASS:null_seen 0 nsec
> test_sock_ops_get_sk:PASS:bug_not_detected 0 nsec
> #419/1 sock_ops_get_sk/get_sk:OK
> run_sock_ops_test:PASS:prog_attach 0 nsec
> run_sock_ops_test:PASS:start_server 0 nsec
> run_sock_ops_test:PASS:connect_to_fd 0 nsec
> test_sock_ops_get_sk:PASS:field_null_seen 0 nsec
> test_sock_ops_get_sk:PASS:field_bug_not_detected 0 nsec
> #419/2 sock_ops_get_sk/get_field:OK
> run_sock_ops_test:PASS:prog_attach 0 nsec
> run_sock_ops_test:PASS:start_server 0 nsec
> run_sock_ops_test:PASS:connect_to_fd 0 nsec
> test_sock_ops_get_sk:PASS:diff_reg_null_seen 0 nsec
> test_sock_ops_get_sk:PASS:diff_reg_bug_not_detected 0 nsec
> #419/3 sock_ops_get_sk/get_sk_diff_reg:OK
> #419 sock_ops_get_sk:OK
> Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
> .../bpf/prog_tests/sock_ops_get_sk.c | 77 ++++++++++++
> .../selftests/bpf/progs/sock_ops_get_sk.c | 117 ++++++++++++++++++
> 2 files changed, 194 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c
> create mode 100644 tools/testing/selftests/bpf/progs/sock_ops_get_sk.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c b/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c
> new file mode 100644
> index 0000000000000..e086f7abb197e
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/sock_ops_get_sk.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <test_progs.h>
> +#include "cgroup_helpers.h"
> +#include "network_helpers.h"
> +#include "sock_ops_get_sk.skel.h"
> +
> +/* See progs/sock_ops_get_sk.c for the bug description. */
> +static void run_sock_ops_test(struct sock_ops_get_sk *skel, int cgroup_fd,
> + int prog_fd)
> +{
> + int server_fd, client_fd, err;
> +
> + err = bpf_prog_attach(prog_fd, cgroup_fd, BPF_CGROUP_SOCK_OPS, 0);
> + if (!ASSERT_OK(err, "prog_attach"))
> + return;
> +
> + server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0);
> + if (!ASSERT_GE(server_fd, 0, "start_server"))
> + goto detach;
> +
> + /* Trigger TCP handshake which causes TCP_NEW_SYN_RECV state where
> + * is_fullsock == 0 and is_locked_tcp_sock == 0.
> + */
> + client_fd = connect_to_fd(server_fd, 0);
> + if (!ASSERT_GE(client_fd, 0, "connect_to_fd"))
> + goto close_server;
> +
> + close(client_fd);
> +
> +close_server:
> + close(server_fd);
> +detach:
> + bpf_prog_detach(cgroup_fd, BPF_CGROUP_SOCK_OPS);
> +}
> +
> +void test_sock_ops_get_sk(void)
> +{
> + struct sock_ops_get_sk *skel;
> + int cgroup_fd;
> +
> + cgroup_fd = test__join_cgroup("/sock_ops_get_sk");
> + if (!ASSERT_GE(cgroup_fd, 0, "join_cgroup"))
> + return;
> +
> + skel = sock_ops_get_sk__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "skel_open_load"))
> + goto close_cgroup;
> +
> + /* Test SOCK_OPS_GET_SK with same src/dst register */
> + if (test__start_subtest("get_sk")) {
> + run_sock_ops_test(skel, cgroup_fd,
> + bpf_program__fd(skel->progs.sock_ops_get_sk_same_reg));
> + ASSERT_EQ(skel->bss->null_seen, 1, "null_seen");
> + ASSERT_EQ(skel->bss->bug_detected, 0, "bug_not_detected");
> + }
> +
> + /* Test SOCK_OPS_GET_FIELD with same src/dst register */
> + if (test__start_subtest("get_field")) {
> + run_sock_ops_test(skel, cgroup_fd,
> + bpf_program__fd(skel->progs.sock_ops_get_field_same_reg));
> + ASSERT_EQ(skel->bss->field_null_seen, 1, "field_null_seen");
> + ASSERT_EQ(skel->bss->field_bug_detected, 0, "field_bug_not_detected");
> + }
> +
> + /* Test SOCK_OPS_GET_SK with different src/dst register */
> + if (test__start_subtest("get_sk_diff_reg")) {
> + run_sock_ops_test(skel, cgroup_fd,
> + bpf_program__fd(skel->progs.sock_ops_get_sk_diff_reg));
> + ASSERT_EQ(skel->bss->diff_reg_null_seen, 1, "diff_reg_null_seen");
> + ASSERT_EQ(skel->bss->diff_reg_bug_detected, 0, "diff_reg_bug_not_detected");
> + }
> +
> + sock_ops_get_sk__destroy(skel);
> +close_cgroup:
> + close(cgroup_fd);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/sock_ops_get_sk.c b/tools/testing/selftests/bpf/progs/sock_ops_get_sk.c
> new file mode 100644
> index 0000000000000..3a0689f8ce7ca
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/sock_ops_get_sk.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +/*
> + * Test the SOCK_OPS_GET_SK() and SOCK_OPS_GET_FIELD() macros in
> + * sock_ops_convert_ctx_access() when dst_reg == src_reg.
> + *
> + * When dst_reg == src_reg, the macros borrow a temporary register to load
> + * is_fullsock / is_locked_tcp_sock, because dst_reg holds the ctx pointer
> + * and cannot be clobbered before ctx->sk / ctx->field is read. If
> + * is_fullsock == 0 (e.g., TCP_NEW_SYN_RECV with a request_sock), the macro
> + * must still zero dst_reg so the verifier's PTR_TO_SOCKET_OR_NULL /
> + * SCALAR_VALUE type is correct at runtime. A missing clear leaves a stale
> + * ctx pointer in dst_reg that passes NULL checks (GET_SK) or leaks a kernel
> + * address as a scalar (GET_FIELD).
> + *
> + * When dst_reg != src_reg, dst_reg itself is used to load is_fullsock, so
> + * the JEQ (dst_reg == 0) naturally leaves it zeroed on the !fullsock path.
> + */
> +
> +int bug_detected;
> +int null_seen;
> +
> +SEC("sockops")
> +__naked void sock_ops_get_sk_same_reg(void)
> +{
> + asm volatile (
> + "r7 = *(u32 *)(r1 + %[is_fullsock_off]);"
> + "r1 = *(u64 *)(r1 + %[sk_off]);"
> + "if r7 != 0 goto 2f;"
> + "if r1 == 0 goto 1f;"
> + "r1 = %[bug_detected] ll;"
> + "r2 = 1;"
> + "*(u32 *)(r1 + 0) = r2;"
> + "goto 2f;"
> + "1:"
> + "r1 = %[null_seen] ll;"
> + "r2 = 1;"
> + "*(u32 *)(r1 + 0) = r2;"
> + "2:"
> + "r0 = 1;"
> + "exit;"
> + :
> + : __imm_const(is_fullsock_off, offsetof(struct bpf_sock_ops, is_fullsock)),
> + __imm_const(sk_off, offsetof(struct bpf_sock_ops, sk)),
> + __imm_addr(bug_detected),
> + __imm_addr(null_seen)
> + : __clobber_all);
> +}
> +
> +/* SOCK_OPS_GET_FIELD: same-register, is_locked_tcp_sock == 0 path. */
> +int field_bug_detected;
> +int field_null_seen;
> +
> +SEC("sockops")
> +__naked void sock_ops_get_field_same_reg(void)
> +{
> + asm volatile (
> + "r7 = *(u32 *)(r1 + %[is_fullsock_off]);"
> + "r1 = *(u32 *)(r1 + %[snd_cwnd_off]);"
> + "if r7 != 0 goto 2f;"
> + "if r1 == 0 goto 1f;"
> + "r1 = %[field_bug_detected] ll;"
> + "r2 = 1;"
> + "*(u32 *)(r1 + 0) = r2;"
> + "goto 2f;"
> + "1:"
> + "r1 = %[field_null_seen] ll;"
> + "r2 = 1;"
> + "*(u32 *)(r1 + 0) = r2;"
> + "2:"
> + "r0 = 1;"
> + "exit;"
> + :
> + : __imm_const(is_fullsock_off, offsetof(struct bpf_sock_ops, is_fullsock)),
> + __imm_const(snd_cwnd_off, offsetof(struct bpf_sock_ops, snd_cwnd)),
> + __imm_addr(field_bug_detected),
> + __imm_addr(field_null_seen)
> + : __clobber_all);
> +}
> +
> +/* SOCK_OPS_GET_SK: different-register, is_fullsock == 0 path. */
> +int diff_reg_bug_detected;
> +int diff_reg_null_seen;
> +
> +SEC("sockops")
> +__naked void sock_ops_get_sk_diff_reg(void)
> +{
> + asm volatile (
> + "r7 = r1;"
> + "r6 = *(u32 *)(r7 + %[is_fullsock_off]);"
> + "r2 = *(u64 *)(r7 + %[sk_off]);"
> + "if r6 != 0 goto 2f;"
> + "if r2 == 0 goto 1f;"
> + "r1 = %[diff_reg_bug_detected] ll;"
> + "r3 = 1;"
> + "*(u32 *)(r1 + 0) = r3;"
> + "goto 2f;"
> + "1:"
> + "r1 = %[diff_reg_null_seen] ll;"
> + "r3 = 1;"
> + "*(u32 *)(r1 + 0) = r3;"
> + "2:"
> + "r0 = 1;"
> + "exit;"
> + :
> + : __imm_const(is_fullsock_off, offsetof(struct bpf_sock_ops, is_fullsock)),
> + __imm_const(sk_off, offsetof(struct bpf_sock_ops, sk)),
> + __imm_addr(diff_reg_bug_detected),
> + __imm_addr(diff_reg_null_seen)
> + : __clobber_all);
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.43.0
>
>
Reviewed-by: Sun Jian <sun.jian.kdev@gmail.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox