* Re: [PATCH net 4/4] gve: Make ethtool config changes synchronous
From: Pin-yen Lin @ 2026-04-24 1:23 UTC (permalink / raw)
To: Paolo Abeni
Cc: Harshitha Ramamurthy, netdev, joshwash, andrew+netdev, davem,
edumazet, kuba, willemb, maolson, nktgrg, jfraker, ziweixiao,
jacob.e.keller, pkaligineedi, shailend, jordanrhee, stable,
linux-kernel
In-Reply-To: <d0981984-b55f-495e-848b-6e9611f0c2ff@redhat.com>
Hi Paolo,
On Thu, Apr 23, 2026 at 6:25 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 4/20/26 7:18 PM, Harshitha Ramamurthy wrote:
> > From: Pin-yen Lin <treapking@google.com>
> >
> > When modifying device features via ethtool, the driver queues the
> > carrier status update to its workqueue (gve_wq). This leads to a
> > short link-down state after running the ethtool command.
> >
> > Use `gve_turnup_and_check_status()` instead of `gve_turnup()` in
> > `gve_queues_start()` to update the carrier status before returning to
> > the userspace.
> >
> > This was discovered by drivers/net/ping.py selftest. The test calls
> > ping command right after an ethtool configuration, but the interface
> > could be down without this fix.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 5f08cd3d6423 ("gve: Alloc before freeing when adjusting queues")
> > Reviewed-by: Joshua Washington <joshwash@google.com>
> > Signed-off-by: Pin-yen Lin <treapking@google.com>
> > Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> > ---
> > drivers/net/ethernet/google/gve/gve_main.c | 56 +++++++++++-----------
> > 1 file changed, 28 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> > index 8617782791e0..d3b4bec38de5 100644
> > --- a/drivers/net/ethernet/google/gve/gve_main.c
> > +++ b/drivers/net/ethernet/google/gve/gve_main.c
> > @@ -1374,6 +1374,33 @@ static void gve_queues_mem_remove(struct gve_priv *priv)
> > priv->rx = NULL;
> > }
> >
> > +static void gve_handle_link_status(struct gve_priv *priv, bool link_status)
> > +{
> > + if (!gve_get_napi_enabled(priv))
> > + return;
> > +
> > + if (link_status == netif_carrier_ok(priv->dev))
> > + return;
> > +
> > + if (link_status) {
> > + netdev_info(priv->dev, "Device link is up.\n");
> > + netif_carrier_on(priv->dev);
> > + } else {
> > + netdev_info(priv->dev, "Device link is down.\n");
> > + netif_carrier_off(priv->dev);
> > + }
> > +}
> > +
> > +static void gve_turnup_and_check_status(struct gve_priv *priv)
> > +{
> > + u32 status;
> > +
> > + gve_turnup(priv);
> > + status = ioread32be(&priv->reg_bar0->device_status);
> > + gve_handle_link_status(priv,
> > + GVE_DEVICE_STATUS_LINK_STATUS_MASK & status);
> > +}
> > +
> > /* The passed-in queue memory is stored into priv and the queues are made live.
> > * No memory is allocated. Passed-in memory is freed on errors.
> > */
> > @@ -1434,8 +1461,7 @@ static int gve_queues_start(struct gve_priv *priv,
> > round_jiffies(jiffies +
> > msecs_to_jiffies(priv->stats_report_timer_period)));
> >
> > - gve_turnup(priv);
> > - queue_work(priv->gve_wq, &priv->service_task);
> > + gve_turnup_and_check_status(priv);
>
> Sashiko says:
>
> Since gve_handle_link_status() can now be called from process context
> via gve_turnup_and_check_status(), while also being concurrently
> executed by gve_service_task() on the workqueue, could this create a
> time-of-check to time-of-use race?
> If the physical link toggles rapidly, could the workqueue thread sample
> the later hardware state (e.g. OFF) but see the software state is
> already OFF and return early, while the process context thread sampled
> the earlier state (e.g. ON), evaluated netif_carrier_ok() as OFF, and
> proceeded to call netif_carrier_on()?
> This might leave the software carrier state stuck ON when the most
> recent hardware state is OFF, because the condition check and update are
> no longer serialized by the workqueue.
>
This is a legitimate concern, but it is not introduced by this series.
gve_turnup_and_check_status() is an existing function. And, given that
there is no locking around gve_service_task(), there could be
concurrency issues on gve_handle_link_status().
We will upload a separate patch to fix this.
> Notes that there more comments:
>
> https://sashiko.dev/#/patchset/20260420171837.455487-1-hramamurthy%40google.com
>
> but I'm not sure if they are actual regressions introduced by this series.
Thanks for the pointers. I'll reply to all the Sashiko comments on the
mailing list.
>
> /P
>
Regards,
Pin-yen
^ permalink raw reply
* Re: [PATCH net 3/4] gve: Use default min ring size when device option values are 0
From: Pin-yen Lin @ 2026-04-24 1:27 UTC (permalink / raw)
To: Harshitha Ramamurthy
Cc: netdev, joshwash, andrew+netdev, davem, edumazet, kuba, pabeni,
willemb, maolson, nktgrg, jfraker, ziweixiao, jacob.e.keller,
pkaligineedi, shailend, jordanrhee, stable, linux-kernel
In-Reply-To: <20260420171837.455487-4-hramamurthy@google.com>
On Mon, Apr 20, 2026 at 10:18 AM Harshitha Ramamurthy
<hramamurthy@google.com> wrote:
>
> From: Pin-yen Lin <treapking@google.com>
>
> On gvnic devices that support reporting minimum ring sizes, the device
> option always includes the min_(rx|tx)_ring_size fields, and the values
> will be 0 if they are not configured to be exposed. This makes the
> driver allow unexpected small ring size configurations from the
> userspace.
>
> Use the default ring size in the driver if the min ring sizes from the
> device option are 0.
>
> This was discovered by drivers/net/ring_reconfig.py selftest.
>
> Cc: stable@vger.kernel.org
> Fixes: ed4fb326947d ("gve: add support to read ring size ranges from the device")
> Reviewed-by: Joshua Washington <joshwash@google.com>
> Reviewed-by: Jordan Rhee <jordanrhee@google.com>
> Signed-off-by: Pin-yen Lin <treapking@google.com>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> ---
> drivers/net/ethernet/google/gve/gve_adminq.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index b72cc0fa2ba2..57d898f6fa82 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -189,7 +189,9 @@ void gve_parse_device_option(struct gve_priv *priv,
> *dev_op_modify_ring = (void *)(option + 1);
>
> /* device has not provided min ring size */
> - if (option_length == GVE_DEVICE_OPTION_NO_MIN_RING_SIZE)
> + if (option_length == GVE_DEVICE_OPTION_NO_MIN_RING_SIZE ||
> + be16_to_cpu((*dev_op_modify_ring)->min_rx_ring_size) == 0 ||
> + be16_to_cpu((*dev_op_modify_ring)->min_tx_ring_size) == 0)
Sashiko says:
Does this introduce an out-of-bounds read regression if the option length
is between 9 and 11?
The length check earlier in the switch case only ensures option_length is at
least GVE_DEVICE_OPTION_NO_MIN_RING_SIZE (which is 8). The new condition
reads min_tx_ring_size, which sits at bytes 10-11 of the structure.
If an option length of 10 is provided, the condition would read past the
provided data length. Could this trigger a page fault if the option payload
happens to be placed at the very end of an unmapped DMA page boundary?
Could this introduce a time-of-check to time-of-use regression? Can these
values be modified by the device between this check and when they are used
later?
Since dev_op_modify_ring points into shared DMA memory, the values are
fetched here for validation. If the device alters the DMA memory to 0 after
this check has completed, would the driver bypass this validation when
applying the values later, leading to a minimum ring size of 0?
Does this introduce a logic regression by discarding a valid configuration
for one ring if the other ring's size is 0?
For example, if min_rx_ring_size is 0 but min_tx_ring_size is explicitly
configured to a valid size like 1024, setting a single default_min_ring_size
flag seems to force both rings to use their default fallback values instead
of evaluating their validity independently.
This should not be an issue because the GVNIC never returns an option
length between 9 and 11. Depending on the GVNIC version, it either
omits the ring sizes or includes both RX and TX ring sizes (even if
the values are 0).
> priv->default_min_ring_size = true;
> break;
> case GVE_DEV_OPT_ID_FLOW_STEERING:
> --
> 2.54.0.rc0.605.g598a273b03-goog
>
Regards,
Pin-yen
^ permalink raw reply
* Re: [PATCH RFC net-next 0/4] net: pse-pd: decouple controller lookup from MDIO probe
From: Corey Leavitt @ 2026-04-24 1:28 UTC (permalink / raw)
To: Jonas Gorski
Cc: Oleksij Rempel, Kory Maincent, Andrew Lunn, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Heiner Kallweit, Russell King, netdev, linux-kernel
In-Reply-To: <c346ac78-3e33-4062-a459-b83d845dc2eb@gmail.com>
Apologies, bad wording in that quick apology. v2 will be a fresh
thread with a Link: trailer to v1, per the docs you linked. Thanks
for flagging it.
Best regards,
Corey
^ permalink raw reply
* Re: [PATCH] devlink/param: replace deprecated strcpy() with strscpy()
From: Jakub Kicinski @ 2026-04-24 1:31 UTC (permalink / raw)
To: Álvaro Costa
Cc: jiri, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
netdev
In-Reply-To: <20260424012037.89619-1-alvaroc.dev@gmail.com>
On Thu, 23 Apr 2026 22:20:22 -0300 Álvaro Costa wrote:
> Replace strcpy() call used to extract a string parameter from param_data
> with strscpy(). Since strscpy() already performs bounds checking and
> ensures the destination string is NUL-terminated, remove the string
> length check as well.
## Form letter - net-next-closed
We have already submitted our pull request with net-next material for v7.1,
and therefore net-next is closed for new drivers, features, code refactoring
and optimizations. We are currently accepting bug fixes only.
Please repost when net-next reopens after Apr 27th.
RFC patches sent for review only are obviously welcome at any time.
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer
pv-bot: closed
^ permalink raw reply
* Re: [GIT PULL] Networking deletions for 7.1
From: Roman Gushchin @ 2026-04-24 1:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kuba, davem, netdev, linux-kernel, Paolo Abeni, Linus Torvalds
In-Reply-To: <20260423235422.1541768-1-kuba@kernel.org>
2c on Sashiko:
1) I'm working on an infrastructure to separate pre-existing issues from
new issues. My current thinking is to stop reporting these issues with
reviews of new patches and instead put them into some database and give
maintainers access to it. Sashiko will automatically deduplicate issues
and index them by the source file/subsystem. Hopefully it will mean that
maintainers will see only a limited number of issues in source files
they support. But I have yet to see how it works in practice.
But I'm somewhat concerned that this way many of these issues will
remain there forever and by reporting them with new material we actually
have better chances to get them fixes. Maybe it should be configurable
per-subsystem. I'm very open for ideas here.
2) Re false positives vs finding more bugs I had the same experience.
It's easy to tweak it to be more conservative or creative, but it comes
at a price. It seems like the real answer is simple a better model. We
saw a big improvement internally switching from Gemini Pro 3.0 to 3.1.
Thanks
^ permalink raw reply
* Re: [PATCH net 2/4] gve: Fix backward stats when interface goes down or configuration is adjusted
From: Pin-yen Lin @ 2026-04-24 1:44 UTC (permalink / raw)
To: Paolo Abeni
Cc: Harshitha Ramamurthy, netdev, joshwash, andrew+netdev, davem,
edumazet, kuba, willemb, maolson, nktgrg, jfraker, ziweixiao,
jacob.e.keller, pkaligineedi, shailend, jordanrhee, stable,
linux-kernel, Debarghya Kundu
In-Reply-To: <CAHwYsirGS1Frgtg+V7D2PB74F5gxAB8GXo+Zvss_aYbJC5LwFg@mail.gmail.com>
On Thu, Apr 23, 2026 at 6:17 PM Pin-yen Lin <treapking@google.com> wrote:
>
> Hi Paolo,
>
> On Thu, Apr 23, 2026 at 4:47 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >
> > On 4/20/26 7:18 PM, Harshitha Ramamurthy wrote:
> > > From: Debarghya Kundu <debarghyak@google.com>
> > >
> > > gve_get_base_stats() sets all the stats to 0, so the stats go backwards
> > > when interface goes down or configuration is adjusted.
> > >
> > > Fix this by persisting baseline stats across interface down.
> > >
> > > This was discovered by drivers/net/stats.py selftest.
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 2e5e0932dff5 ("gve: add support for basic queue stats")
> > > Signed-off-by: Debarghya Kundu <debarghyak@google.com>
> > > Signed-off-by: Pin-yen Lin <treapking@google.com>
> > > Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> > > ---
> > > drivers/net/ethernet/google/gve/gve.h | 6 ++
> > > drivers/net/ethernet/google/gve/gve_main.c | 64 +++++++++++++++++++---
> > > 2 files changed, 63 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
> > > index cbdf3a842cfe..ff7797043908 100644
> > > --- a/drivers/net/ethernet/google/gve/gve.h
> > > +++ b/drivers/net/ethernet/google/gve/gve.h
> > > @@ -794,6 +794,10 @@ struct gve_ptp {
> > > struct gve_priv *priv;
> > > };
> > >
> > > +struct gve_ring_err_stats {
> > > + u64 rx_alloc_fails;
> > > +};
> > > +
> > > struct gve_priv {
> > > struct net_device *dev;
> > > struct gve_tx_ring *tx; /* array of tx_cfg.num_queues */
> > > @@ -882,6 +886,8 @@ struct gve_priv {
> > > unsigned long service_task_flags;
> > > unsigned long state_flags;
> > >
> > > + struct gve_ring_err_stats base_ring_err_stats;
> > > + struct rtnl_link_stats64 base_net_stats;
> > > struct gve_stats_report *stats_report;
> > > u64 stats_report_len;
> > > dma_addr_t stats_report_bus; /* dma address for the stats report */
> > > diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> > > index 675382e9756c..8617782791e0 100644
> > > --- a/drivers/net/ethernet/google/gve/gve_main.c
> > > +++ b/drivers/net/ethernet/google/gve/gve_main.c
> > > @@ -105,9 +105,22 @@ static netdev_tx_t gve_start_xmit(struct sk_buff *skb, struct net_device *dev)
> > > return gve_tx_dqo(skb, dev);
> > > }
> > >
> > > -static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s)
> > > +static void gve_add_base_stats(struct gve_priv *priv,
> > > + struct rtnl_link_stats64 *s)
> > > +{
> > > + struct rtnl_link_stats64 *base_stats = &priv->base_net_stats;
> > > +
> > > + s->rx_packets += base_stats->rx_packets;
> > > + s->rx_bytes += base_stats->rx_bytes;
> > > + s->rx_dropped += base_stats->rx_dropped;
> > > + s->tx_packets += base_stats->tx_packets;
> > > + s->tx_bytes += base_stats->tx_bytes;
> > > + s->tx_dropped += base_stats->tx_dropped;
> > > +}
>
> Sashiko says:
>
> Can this result in torn reads on 32-bit architectures?
> The base_net_stats struct accumulates 64-bit network statistics in gve_close()
> under rtnl_lock, but these stats are read here via ndo_get_stats64 which can
> execute concurrently without rtnl_lock.
> On 32-bit systems, a concurrent update might result in torn reads since 64-bit
> memory reads are not atomic.
> Should u64_stats_sync sequence counters or atomic types be used here?
>
> We will add u64_stats_fetch_(begin|retry) to guard this in v2.
>
> > > +
> > > +static void gve_get_ring_stats(struct gve_priv *priv,
> > > + struct rtnl_link_stats64 *s)
> > > {
> > > - struct gve_priv *priv = netdev_priv(dev);
> > > unsigned int start;
> > > u64 packets, bytes;
> > > int num_tx_queues;
> > > @@ -142,6 +155,14 @@ static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s)
> > > }
> > > }
> > >
> > > +static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s)
> > > +{
> > > + struct gve_priv *priv = netdev_priv(dev);
> > > +
> > > + gve_get_ring_stats(priv, s);
> > > + gve_add_base_stats(priv, s);
> > > +}
> > > +
> > > static int gve_alloc_flow_rule_caches(struct gve_priv *priv)
> > > {
> > > struct gve_flow_rules_cache *flow_rules_cache = &priv->flow_rules_cache;
> > > @@ -1493,6 +1514,23 @@ static int gve_queues_stop(struct gve_priv *priv)
> > > return gve_reset_recovery(priv, false);
> > > }
> > >
> > > +static void gve_get_ring_err_stats(struct gve_priv *priv,
> > > + struct gve_ring_err_stats *err_stats)
> > > +{
> > > + int ring;
Another Sashiko comment:
If the interface is brought down when ring memory allocation previously
failed, could priv->rx be NULL here?
Unlike gve_get_ring_stats(), there is no check for whether priv->rx is
allocated before dereferencing it.
This will also be fixed in v2.
> > > +
> > > + for (ring = 0; ring < priv->rx_cfg.num_queues; ring++) {
> > > + unsigned int start;
> > > + struct gve_rx_ring *rx = &priv->rx[ring];
> > > +
> > > + do {
> > > + start = u64_stats_fetch_begin(&rx->statss);
> > > + err_stats->rx_alloc_fails +=
> > > + rx->rx_skb_alloc_fail + rx->rx_buf_alloc_fail;
> > > + } while (u64_stats_fetch_retry(&rx->statss, start));
> >
> > Sashiko says:
> >
> > Could this loop improperly inflate the baseline metric by double counting?
> > If a concurrent update causes the sequence counter to change,
> > u64_stats_fetch_retry() forces the loop to restart. Because the addition
> > is performed in-place on err_stats->rx_alloc_fails, the same ring's
> > error values will be added again.
> > Would it be safer to use local variables inside the retry loop and update
> > the global accumulator only after the loop completes successfully, similar
> > to the pattern established in gve_get_ring_stats()?
> >
> We'll fix this by using local variables in v2.
> >
> > > + }
> > > +}
> > > +
> > > static int gve_close(struct net_device *dev)
> > > {
> > > struct gve_priv *priv = netdev_priv(dev);
> > > @@ -1502,6 +1540,10 @@ static int gve_close(struct net_device *dev)
> > > if (err)
> > > return err;
> > >
> > > + /* Save ring queue and err stats before closing the interface */
> > > + gve_get_ring_stats(priv, &priv->base_net_stats);
> > > + gve_get_ring_err_stats(priv, &priv->base_ring_err_stats);
> >
> > Sashiko says:
> >
> > Does this create a temporary spike in reported statistics?
> > During gve_close(), the active ring stats are added to base_net_stats.
> > However, priv->rx and priv->tx are not set to NULL until the memory
> > teardown completes in gve_queues_mem_remove().
> > If ndo_get_stats64 is called concurrently during this window, it will
> > add both the active ring stats and the newly updated base stats together.
> > This causes the reported statistics to temporarily double until the
> > teardown finishes.
>
> As Sashiko mentioned, this is an existing concurrency issue between
> gve_close() and gve_get_stats() (i.e., .ndo_get_stats64() callback of
> gve driver):
>
> gve_queues_mem_remove(priv);
> This is a pre-existing issue, but can this lead to a use-after-free
> during concurrent stats retrieval?
> When gve_queues_mem_remove() frees the ring memory, priv->rx and
> priv->tx are not set to NULL until after the memory is freed. If
> gve_get_stats() executes during this window, it may iterate over and
> dereference the already freed ring memory.
>
> We will send out a separate patch to fix this.
>
> >
> > Note that you are expected to proactively comment/reply on the ML to the
> > concerns raised by sashiko reviews.
>
> Thanks for the reminder.
>
> >
> > Thanks,
> >
> > Paolo
> >
>
> Regards,
> Pin-yen
Regards,
Pin-yen
^ permalink raw reply
* [PATCH net v2] tcp: make probe0 timer handle expired user timeout
From: Altan Hacigumus @ 2026-04-24 1:46 UTC (permalink / raw)
To: Eric Dumazet, Neal Cardwell, Kuniyuki Iwashima, David S . Miller,
David Ahern, Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: netdev, linux-kernel, Enke Chen
In-Reply-To: <20260414013634.43997-1-ahacigu.linux@gmail.com>
tcp_clamp_probe0_to_user_timeout() computes remaining time in jiffies
using subtraction with an unsigned lvalue. If elapsed probing time
exceeds the configured TCP_USER_TIMEOUT, the underflow yields a large
value.
This ends up re-arming the probe timer for a full backoff interval
instead of expiring immediately, delaying connection teardown beyond
the configured timeout.
Fix this by preventing underflow so user-set timeout expiration is
handled correctly without extending the probe timer.
Fixes: 344db93ae3ee ("tcp: make TCP_USER_TIMEOUT accurate for zero window probes")
Link: https://lore.kernel.org/r/20260414013634.43997-1-ahacigu.linux@gmail.com
Signed-off-by: Altan Hacigumus <ahacigu.linux@gmail.com>
---
v2:
- Use the existing TCP_TIMEOUT_MIN clamping along with signed @remaining
---
net/ipv4/tcp_timer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 8d791a954cd6..322db13333c7 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -50,7 +50,8 @@ static u32 tcp_clamp_rto_to_user_timeout(const struct sock *sk)
u32 tcp_clamp_probe0_to_user_timeout(const struct sock *sk, u32 when)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
- u32 remaining, user_timeout;
+ u32 user_timeout;
+ s32 remaining;
s32 elapsed;
user_timeout = READ_ONCE(icsk->icsk_user_timeout);
@@ -61,7 +62,7 @@ u32 tcp_clamp_probe0_to_user_timeout(const struct sock *sk, u32 when)
if (unlikely(elapsed < 0))
elapsed = 0;
remaining = msecs_to_jiffies(user_timeout) - elapsed;
- remaining = max_t(u32, remaining, TCP_TIMEOUT_MIN);
+ remaining = max_t(int, remaining, TCP_TIMEOUT_MIN);
return min_t(u32, remaining, when);
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v2] netlink: clean up failed initial dump-start state
From: Jakub Kicinski @ 2026-04-24 1:50 UTC (permalink / raw)
To: Michael Bommarito
Cc: David S . Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Kuniyuki Iwashima, Kees Cook, Feng Yang, netdev, linux-kernel
In-Reply-To: <20260423212827.1177552-1-michael.bommarito@gmail.com>
On Thu, 23 Apr 2026 17:28:27 -0400 Michael Bommarito wrote:
> __netlink_dump_start() installs cb->skb, takes the module reference,
> and sets cb_running before calling netlink_dump(sk, true). If that
> first call returns via errout_skb the callback state is left behind:
> cb_running stays set, module_put() and consume_skb(cb->skb) are
> deferred until recvmsg() drives the dump back through the success
> path, or netlink_release() on close runs the catch-all cleanup. On
> sustained alloc failure neither fires.
>
> Factor the teardown into netlink_dump_cleanup(nlk, drop) shared by
> the dump success path, the lock_taken=true errout_skb path, and
> netlink_release(). The @drop flag preserves the existing split:
> consume_skb() on normal completion, kfree_skb() on abort.
>
> Validation on a UML guest: an unprivileged task opens NETLINK_ROUTE,
> preloads sk_rmem_alloc, then issues RTM_GETLINK | NLM_F_DUMP. Stock
> kernel leaves cb_running stuck at 1 until recvmsg() or close()
> drives it. Patched kernel clears cb_running immediately on the
> lock_taken=true failure; the recvmsg continuation path is unchanged.
>
> At scale: 3500 wedged sockets in a 256M guest show about 3.8-3.9
> MiB of extra unreclaimable slab (~1.1 KiB/sock) on stock vs zero on
> patched. RLIMIT_NOFILE bounds the test before OOM, so this is a
> local availability cleanup rather than an exhaustion primitive.
>
> Fixes: 16b304f3404f ("netlink: Eliminate kmalloc in netlink dump operation.")
Looks like this changes existing behavior :(
The tools/testing/selftests/net/netlink-dumps.c test used to always
see a NLMSG_DONE now it doesn't. Maybe this change is more risk
than reward?
process nit: please don't post new versions in reply to previous
^ permalink raw reply
* [PATCH net v2] net: usb: rtl8150: free skb on usb_submit_urb() failure in xmit
From: Morduan Zang @ 2026-04-24 1:55 UTC (permalink / raw)
To: Jakub Kicinski, Petko Manolov
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Paolo Abeni,
linux-usb, netdev, linux-kernel, Morduan Zang, Andrew Lunn
In-Reply-To: <678BC10BB9E39322+20260421111025.15833-1-zhangdandan@uniontech.com>
When rtl8150_start_xmit() fails to submit the tx URB, the URB is never
handed to the USB core and write_bulk_callback() will not run. The
driver returns NETDEV_TX_OK, which tells the networking stack that the
skb has been consumed, but nothing actually frees the skb on this
error path:
dev->tx_skb = skb;
...
if ((res = usb_submit_urb(dev->tx_urb, GFP_ATOMIC))) {
...
/* no kfree_skb here */
}
return NETDEV_TX_OK;
This leaks the skb on every submit failure and also leaves dev->tx_skb
pointing at memory that the driver itself may later free, which is
fragile.
Free the skb with dev_kfree_skb_any() in the error path and clear
dev->tx_skb so no stale pointer is left behind.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
---
Changes in v2:
- Rebase on net/main as requested (Jakub Kicinski). v1 was based on
a tree that also carried the pending UAF fix ("net: usb: rtl8150:
fix use-after-free in rtl8150_start_xmit()"), so v1 did not apply
on net/main. v2 is an independent fix that applies cleanly to
net/main on its own; it does not depend on the UAF fix being
applied first.
- No code change besides the rebase context.
v1: https://lore.kernel.org/all/678BC10BB9E39322+20260421111025.15833-1-zhangdandan@uniontech.com/
---
drivers/net/usb/rtl8150.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 4cda0643afb6..9999484d2c5e 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -707,6 +707,13 @@ static netdev_tx_t rtl8150_start_xmit(struct sk_buff *skb,
netdev->stats.tx_errors++;
netif_start_queue(netdev);
}
+ /*
+ * The URB was not submitted, so write_bulk_callback() will
+ * never run to free dev->tx_skb. Drop the skb here and
+ * clear tx_skb to avoid leaving a stale pointer.
+ */
+ dev->tx_skb = NULL;
+ dev_kfree_skb_any(skb);
} else {
netdev->stats.tx_packets++;
netdev->stats.tx_bytes += skb->len;
--
2.50.1
^ permalink raw reply related
* [syzbot] [virt?] [net?] memory leak in __vsock_create (2)
From: syzbot @ 2026-04-24 2:02 UTC (permalink / raw)
To: davem, edumazet, horms, kuba, linux-kernel, netdev, pabeni,
sgarzare, syzkaller-bugs, virtualization
Hello,
syzbot found the following issue on:
HEAD commit: 1f318b96cc84 Linux 7.0-rc3
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14db28ba580000
kernel config: https://syzkaller.appspot.com/x/.config?x=2c6ad6fefffa76b1
dashboard link: https://syzkaller.appspot.com/bug?extid=1b2c9c4a0f8708082678
compiler: gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12db28ba580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1215175a580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/d4ebd240d832/disk-1f318b96.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/9548f49a7dec/vmlinux-1f318b96.xz
kernel image: https://storage.googleapis.com/syzbot-assets/4e16f2bcfc7d/bzImage-1f318b96.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+1b2c9c4a0f8708082678@syzkaller.appspotmail.com
2026/03/09 23:37:29 executed programs: 5
BUG: memory leak
unreferenced object 0xffff888128bb6d00 (size 1272):
comm "kworker/0:1", pid 10, jiffies 4294944453
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
28 00 0b 40 00 00 00 00 00 00 00 00 00 00 00 00 (..@............
backtrace (crc 31367ed9):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4547 [inline]
slab_alloc_node mm/slub.c:4869 [inline]
kmem_cache_alloc_noprof+0x372/0x480 mm/slub.c:4876
sk_prot_alloc+0x3e/0x1b0 net/core/sock.c:2239
sk_alloc+0x36/0x460 net/core/sock.c:2301
__vsock_create.constprop.0+0x38/0x2f0 net/vmw_vsock/af_vsock.c:893
virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1557 [inline]
virtio_transport_recv_pkt+0x855/0xfc0 net/vmw_vsock/virtio_transport_common.c:1685
vsock_loopback_work+0x104/0x140 net/vmw_vsock/vsock_loopback.c:142
process_one_work+0x26c/0x5d0 kernel/workqueue.c:3275
process_scheduled_works kernel/workqueue.c:3358 [inline]
worker_thread+0x243/0x490 kernel/workqueue.c:3439
kthread+0x14e/0x1a0 kernel/kthread.c:436
ret_from_fork+0x23c/0x4b0 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
BUG: memory leak
unreferenced object 0xffff8881141e93a0 (size 32):
comm "kworker/0:1", pid 10, jiffies 4294944453
hex dump (first 32 bytes):
f8 6e 0a 00 81 88 ff ff 00 00 00 00 00 00 00 00 .n..............
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc e7f5c8b3):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4547 [inline]
slab_alloc_node mm/slub.c:4869 [inline]
__do_kmalloc_node mm/slub.c:5262 [inline]
__kmalloc_noprof+0x3bd/0x560 mm/slub.c:5275
kmalloc_noprof include/linux/slab.h:954 [inline]
kzalloc_noprof include/linux/slab.h:1188 [inline]
lsm_blob_alloc+0x4d/0x80 security/security.c:192
lsm_sock_alloc security/security.c:4375 [inline]
security_sk_alloc+0x2d/0x290 security/security.c:4391
sk_prot_alloc+0x8f/0x1b0 net/core/sock.c:2248
sk_alloc+0x36/0x460 net/core/sock.c:2301
__vsock_create.constprop.0+0x38/0x2f0 net/vmw_vsock/af_vsock.c:893
virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1557 [inline]
virtio_transport_recv_pkt+0x855/0xfc0 net/vmw_vsock/virtio_transport_common.c:1685
vsock_loopback_work+0x104/0x140 net/vmw_vsock/vsock_loopback.c:142
process_one_work+0x26c/0x5d0 kernel/workqueue.c:3275
process_scheduled_works kernel/workqueue.c:3358 [inline]
worker_thread+0x243/0x490 kernel/workqueue.c:3439
kthread+0x14e/0x1a0 kernel/kthread.c:436
ret_from_fork+0x23c/0x4b0 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
BUG: memory leak
unreferenced object 0xffff888128bfa8a0 (size 96):
comm "kworker/0:1", pid 10, jiffies 4294944453
hex dump (first 32 bytes):
00 6d bb 28 81 88 ff ff 00 00 00 00 00 00 00 00 .m.(............
00 00 00 00 00 00 00 00 00 00 04 00 00 00 00 00 ................
backtrace (crc 1fd60988):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4547 [inline]
slab_alloc_node mm/slub.c:4869 [inline]
__kmalloc_cache_noprof+0x377/0x480 mm/slub.c:5378
kmalloc_noprof include/linux/slab.h:950 [inline]
kzalloc_noprof include/linux/slab.h:1188 [inline]
virtio_transport_do_socket_init+0x2b/0xf0 net/vmw_vsock/virtio_transport_common.c:923
vsock_assign_transport+0x309/0x3a0 net/vmw_vsock/af_vsock.c:642
virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1575 [inline]
virtio_transport_recv_pkt+0x8bc/0xfc0 net/vmw_vsock/virtio_transport_common.c:1685
vsock_loopback_work+0x104/0x140 net/vmw_vsock/vsock_loopback.c:142
process_one_work+0x26c/0x5d0 kernel/workqueue.c:3275
process_scheduled_works kernel/workqueue.c:3358 [inline]
worker_thread+0x243/0x490 kernel/workqueue.c:3439
kthread+0x14e/0x1a0 kernel/kthread.c:436
ret_from_fork+0x23c/0x4b0 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
BUG: memory leak
unreferenced object 0xffff888129e1e800 (size 1272):
comm "kworker/1:3", pid 5832, jiffies 4294944454
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
28 00 0b 40 00 00 00 00 00 00 00 00 00 00 00 00 (..@............
backtrace (crc 4bfb7c57):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4547 [inline]
slab_alloc_node mm/slub.c:4869 [inline]
kmem_cache_alloc_noprof+0x372/0x480 mm/slub.c:4876
sk_prot_alloc+0x3e/0x1b0 net/core/sock.c:2239
sk_alloc+0x36/0x460 net/core/sock.c:2301
__vsock_create.constprop.0+0x38/0x2f0 net/vmw_vsock/af_vsock.c:893
virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1557 [inline]
virtio_transport_recv_pkt+0x855/0xfc0 net/vmw_vsock/virtio_transport_common.c:1685
vsock_loopback_work+0x104/0x140 net/vmw_vsock/vsock_loopback.c:142
process_one_work+0x26c/0x5d0 kernel/workqueue.c:3275
process_scheduled_works kernel/workqueue.c:3358 [inline]
worker_thread+0x243/0x490 kernel/workqueue.c:3439
kthread+0x14e/0x1a0 kernel/kthread.c:436
ret_from_fork+0x23c/0x4b0 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
BUG: memory leak
unreferenced object 0xffff888114372e80 (size 32):
comm "kworker/1:3", pid 5832, jiffies 4294944454
hex dump (first 32 bytes):
f8 6e 0a 00 81 88 ff ff 00 00 00 00 00 00 00 00 .n..............
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc e7f5c8b3):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4547 [inline]
slab_alloc_node mm/slub.c:4869 [inline]
__do_kmalloc_node mm/slub.c:5262 [inline]
__kmalloc_noprof+0x3bd/0x560 mm/slub.c:5275
kmalloc_noprof include/linux/slab.h:954 [inline]
kzalloc_noprof include/linux/slab.h:1188 [inline]
lsm_blob_alloc+0x4d/0x80 security/security.c:192
lsm_sock_alloc security/security.c:4375 [inline]
security_sk_alloc+0x2d/0x290 security/security.c:4391
sk_prot_alloc+0x8f/0x1b0 net/core/sock.c:2248
sk_alloc+0x36/0x460 net/core/sock.c:2301
__vsock_create.constprop.0+0x38/0x2f0 net/vmw_vsock/af_vsock.c:893
virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1557 [inline]
virtio_transport_recv_pkt+0x855/0xfc0 net/vmw_vsock/virtio_transport_common.c:1685
vsock_loopback_work+0x104/0x140 net/vmw_vsock/vsock_loopback.c:142
process_one_work+0x26c/0x5d0 kernel/workqueue.c:3275
process_scheduled_works kernel/workqueue.c:3358 [inline]
worker_thread+0x243/0x490 kernel/workqueue.c:3439
kthread+0x14e/0x1a0 kernel/kthread.c:436
ret_from_fork+0x23c/0x4b0 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
BUG: memory leak
unreferenced object 0xffff8881054d0ea0 (size 96):
comm "kworker/1:3", pid 5832, jiffies 4294944454
hex dump (first 32 bytes):
00 e8 e1 29 81 88 ff ff 00 00 00 00 00 00 00 00 ...)............
00 00 00 00 00 00 00 00 00 00 04 00 00 00 00 00 ................
backtrace (crc 72501265):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4547 [inline]
slab_alloc_node mm/slub.c:4869 [inline]
__kmalloc_cache_noprof+0x377/0x480 mm/slub.c:5378
kmalloc_noprof include/linux/slab.h:950 [inline]
kzalloc_noprof include/linux/slab.h:1188 [inline]
virtio_transport_do_socket_init+0x2b/0xf0 net/vmw_vsock/virtio_transport_common.c:923
vsock_assign_transport+0x309/0x3a0 net/vmw_vsock/af_vsock.c:642
virtio_transport_recv_listen net/vmw_vsock/virtio_transport_common.c:1575 [inline]
virtio_transport_recv_pkt+0x8bc/0xfc0 net/vmw_vsock/virtio_transport_common.c:1685
vsock_loopback_work+0x104/0x140 net/vmw_vsock/vsock_loopback.c:142
process_one_work+0x26c/0x5d0 kernel/workqueue.c:3275
process_scheduled_works kernel/workqueue.c:3358 [inline]
worker_thread+0x243/0x490 kernel/workqueue.c:3439
kthread+0x14e/0x1a0 kernel/kthread.c:436
ret_from_fork+0x23c/0x4b0 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
connection error: failed to recv *flatrpc.ExecutorMessageRawT: EOF
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* Re: [PATCH net v2] net: iptunnel: fix stale transport header after GRE/TEB decap
From: Jiayuan Chen @ 2026-04-24 2:04 UTC (permalink / raw)
To: Paolo Abeni, Eric Dumazet
Cc: netdev, syzbot+83181a31faf9455499c5, David S. Miller, David Ahern,
Jakub Kicinski, Simon Horman, Pravin B Shelar, Tom Herbert,
linux-kernel
In-Reply-To: <4f467b95-d135-4c1a-9f44-09138ff2d592@redhat.com>
On 4/23/26 4:19 PM, Paolo Abeni wrote:
> On 4/19/26 3:01 PM, Jiayuan Chen wrote:
>> [...]
>>>> +662,18 @@ static inline int iptunnel_pull_offloads(struct sk_buff *skb)
>>>> return 0;
>>>> }
>>>>
>>>> +static inline void iptunnel_rebuild_transport_header(struct sk_buff *skb)
>>>> +{
>>>> + if (!skb_is_gso(skb))
>>>> + return;
>>>> +
>>>> + skb->transport_header = (typeof(skb->transport_header))~0U;
>>>> + skb_probe_transport_header(skb);
>>>> +
>>>> + if (!skb_transport_header_was_set(skb))
>>>> + skb_gso_reset(skb);
>>> I do not think this makes sense.
>>> What is a valid case for this packet being processed further?
>>> The buggy packet must be dropped, instead of being mangled like this.
>> Hi Eric,
>>
>> The reproducer builds a gre frame whose inner Ethernet header is
>> all-zero. Tracing the skb through RX:
>>
>> 1. At GRE decap exit, skb_transport_offset(skb) < 0 is the rule, not the
>> exception.
>>
>> It is negative for every packet leaving the tunnel, including perfectly
>> well-formed inner IPv4 traffic
>> because the tunnel leaves skb->transport_header at the outer L4 offset while
>> pskb_pull() has already advanced skb->data past it.
> Is it? the transport header is an offset on top of skb->head, pskb_pull
> changes head only if the header is not in the linear part (and the
> transport offset is already invalid).
Sorry, my wording was imprecise. The point is not that `transport_header`
itself holds a negative value — it does not — but that after GRE processing,
`skb->data` has advanced past the outer L4 while `skb->transport_header`
is never touched, so `skb_transport_offset(skb)` ends up negative.
>> skb_transport_header_was_set() stays true, so downstream
>> code that trusts that flag now trusts a stale, negative offset.
>>
>> 2. GRO repairs it — but only for protocols it knows.
>>
>> In dev_gro_receive(), skb->protocol is dispatched through the offload
>> table. For ETH_P_IP,
>> inet_gro_receive() calls skb_set_transport_header(skb,
>> skb_gro_offset(skb)), and the offset
>> becomes valid again. But for malformed skb, dev_gro_receive just bypass it.
> So only malformed packets cause trouble, right?
>
The negative offset is produced for every packet leaving GRE, not just
malformed ones. What differs is what happens downstream:
- For well-formed inner IPv4, `inet_gro_receive()` calls
`skb_set_transport_header(skb, skb_gro_offset(skb))` and restores a
valid offset before any consumer observes it.
- For malformed inner frames (e.g. `skb->protocol == ETH_P_802_2 or
other `),
`dev_gro_receive()` finds no ptype and just passes the skb through.
The stale negative offset survives into `__netif_receive_skb_core()`.
So the UAF needs both conditions: GRE producing the stale offset *and*
no downstream rescue.
>> 3. Both kinds then reach __netif_receive_skb_core().
>>
>> So the skb that qdisc/tc/BPF segmenters later see has an
>> invariant violation — _was_set == true but offset < 0 — that the core
>> layer has no intention of catching for us.
>>
>> My reading of this is that the tunnel decap path is producing an skb
>> that doesn't
>> honor the contract __netif_receive_skb_core() expects from its
>> producers, and that
>> it doesn't really make sense to ask GRE to parse or validate the inner
>> L4 in order
>> to fix this.
>>
>> I'm thinking at the end of GRE decap, before handing the skb to
>> gro_cells_receive(),
>> call skb_reset_transport_header(skb).
> My take is that you need to address the issue earlier than the current
> patch, dropping the malformed packets.
>
> /P
Dropping at tunnel decap is a reasonable option, e.g.:
if (unlikely(skb->protocol == htons(ETH_P_802_2) ||
skb->protocol == htons(ETH_P_802_3) ||
....)) {
kfree_skb_reason(skb, SKB_DROP_REASON_...);
return 0;
}
Two concerns about this approach, though:
1.It asks GRE to decide whether an inner L2 frame is "sensible",
which I don't think should be GRE's responsibility — GRE is a
generic L2/L3 tunnel and historically stays agnostic about the
inner payload.
2. More importantly, filtering on ETH_P_802_2 / ETH_P_802_3 only
covers the case where inner h_proto < ETH_P_802_3_MIN. The same
stale-offset condition can also be reached with any inner
ethertype that has no GRO receive callback resetting
transport_header.
In my earlier reply to Eric I suggested calling
skb_reset_transport_header(skb) at the tunnel decap exit instead.
A few reasons I think this is a cleaner fix:
1.It is inner-protocol agnostic — it normalizes the skb regardless
of what the inner ethertype happens to be, so ARP/PPPoE/... are
fixed by the same one-liner.
2.ip_tunnel_rcv() already updates mac_header (via eth_type_trans)
and network_header (ip_tunnel.c:414). transport_header is the
only one of the three left pointing at the outer offset; resetting
it here is completing what the function is already doing for the
other two.
3.Malformed frames that carry it downstream simply fail ptype_base
dispatch and are dropped there, the same way any unknown-ethertype
frame is dropped today.
^ permalink raw reply
* [PATCH net] net: enetc: fix VSI mailbox timeout handling and DMA lifecycle
From: Wei Fang @ 2026-04-24 2:15 UTC (permalink / raw)
To: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
davem, edumazet, kuba, pabeni
Cc: netdev, linux-kernel, imx
In the current VSI mailbox implementation, the VSI allocates a DMA buffer
to store the message sent to the PSI. When the PSI receives the message
request from the VSI, the hardware transfers the message from this DMA
buffer to PSI's DMA buffer for processing.
When enetc_msg_vsi_send() times out, two scenarios can occur:
1) Use-after-free: If the hardware hasn't completed DMA transfer when
the VSI frees the buffer, the hardware may subsequently transfer the
data from freed memory to PSI's DMA buffer, causing memory corruption.
2) Message race: If PSI hasn't processed the previous message when the
next message is sent, the VSI may receive the previous message's
reply, leading to incorrect handling.
To address these issues, implement the following changes:
- Check the mailbox busy status before sending a new message. If the
mailbox is in busy state, it indicates the previous message is still
being processed, so return -EBUSY error immediately.
- Add the 'msg' field to struct enetc_si to preserve the DMA buffer
information. The caller of enetc_msg_vsi_send() no longer frees the
DMA buffer. Instead, defer freeing until it is safe to do so (when
mailbox is not busy on next send).
- Add cleanup in enetc_vf_remove() to free the last message buffer.
This ensures the DMA buffer remains valid during hardware transfer and
prevents message reply mismatches.
Fixes: beb74ac878c8 ("enetc: Add vf to pf messaging support")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
drivers/net/ethernet/freescale/enetc/enetc.h | 1 +
.../net/ethernet/freescale/enetc/enetc_vf.c | 41 +++++++++++++++----
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index e663bb5e614e..e691144e8756 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -330,6 +330,7 @@ struct enetc_si {
struct workqueue_struct *workqueue;
struct work_struct rx_mode_task;
struct dentry *debugfs_root;
+ struct enetc_msg_swbd msg; /* Only valid for VSI */
};
#define ENETC_SI_ALIGN 32
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
index 6c4b374bcb0e..7cd05e9bdfe4 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
@@ -17,11 +17,36 @@ static void enetc_msg_vsi_write_msg(struct enetc_hw *hw,
enetc_wr(hw, ENETC_VSIMSGSNDAR0, val);
}
+static void enetc_msg_dma_free(struct device *dev, struct enetc_msg_swbd *msg)
+{
+ if (msg->vaddr) {
+ dma_free_coherent(dev, msg->size, msg->vaddr, msg->dma);
+ msg->vaddr = NULL;
+ }
+}
+
static int enetc_msg_vsi_send(struct enetc_si *si, struct enetc_msg_swbd *msg)
{
+ struct device *dev = &si->pdev->dev;
int timeout = 100;
u32 vsimsgsr;
+ /* The VSI mailbox may be busy if last message was not yet processed
+ * by PSI. So need to check the mailbox status before sending.
+ */
+ vsimsgsr = enetc_rd(&si->hw, ENETC_VSIMSGSR);
+ if (vsimsgsr & ENETC_VSIMSGSR_MB) {
+ /* It is safe to free the DMA buffer here, the caller does
+ * not access the DMA buffer if enetc_msg_vsi_send() fails.
+ */
+ enetc_msg_dma_free(dev, msg);
+ dev_err(dev, "VSI mailbox is busy\n");
+ return -EBUSY;
+ }
+
+ /* Free the DMA buffer of the last message */
+ enetc_msg_dma_free(dev, &si->msg);
+ si->msg = *msg;
enetc_msg_vsi_write_msg(&si->hw, msg);
do {
@@ -32,12 +57,15 @@ static int enetc_msg_vsi_send(struct enetc_si *si, struct enetc_msg_swbd *msg)
usleep_range(1000, 2000);
} while (--timeout);
- if (!timeout)
+ if (!timeout) {
+ dev_err(dev, "VSI mailbox timeout\n");
+
return -ETIMEDOUT;
+ }
/* check for message delivery error */
if (vsimsgsr & ENETC_VSIMSGSR_MS) {
- dev_err(&si->pdev->dev, "VSI command execute error: %d\n",
+ dev_err(dev, "VSI command execute error: %d\n",
ENETC_SIMSGSR_GET_MC(vsimsgsr));
return -EIO;
}
@@ -50,7 +78,6 @@ static int enetc_msg_vsi_set_primary_mac_addr(struct enetc_ndev_priv *priv,
{
struct enetc_msg_cmd_set_primary_mac *cmd;
struct enetc_msg_swbd msg;
- int err;
msg.size = ALIGN(sizeof(struct enetc_msg_cmd_set_primary_mac), 64);
msg.vaddr = dma_alloc_coherent(priv->dev, msg.size, &msg.dma,
@@ -67,11 +94,7 @@ static int enetc_msg_vsi_set_primary_mac_addr(struct enetc_ndev_priv *priv,
memcpy(&cmd->mac, saddr, sizeof(struct sockaddr));
/* send the command and wait */
- err = enetc_msg_vsi_send(priv->si, &msg);
-
- dma_free_coherent(priv->dev, msg.size, msg.vaddr, msg.dma);
-
- return err;
+ return enetc_msg_vsi_send(priv->si, &msg);
}
static int enetc_vf_set_mac_addr(struct net_device *ndev, void *addr)
@@ -269,7 +292,7 @@ static void enetc_vf_remove(struct pci_dev *pdev)
enetc_teardown_cbdr(&si->cbd_ring);
free_netdev(si->ndev);
-
+ enetc_msg_dma_free(&pdev->dev, &si->msg);
enetc_pci_remove(pdev);
}
--
2.34.1
^ permalink raw reply related
* Re: [RESEND PATCH] connector/Kconfig: Enable CONFIG_CONNECTOR by default
From: zhidao su @ 2026-04-24 2:28 UTC (permalink / raw)
To: qyousef
Cc: mingo, peterz, andrew+netdev, davem, edumazet, kuba, netdev,
linux-kernel, vincent.guittot, jstultz, rostedt, pabeni
In-Reply-To: <20260420230630.205527-1-qyousef@layalina.io>
On 04/20/26 23:04, Qais Yousef wrote:
> This seems widely enabled feature by distro in general.
From testing on an Android device: schedqos depends on CONFIG_CONNECTOR to
receive NETLINK proc events for both native binaries and Java app processes.
Without it, process tracking silently doesn't work.
Not sure if this is useful context for the decision, but wanted to share in
case it helps.
Best regards,
Zhidao Su
^ permalink raw reply
* Re: [GIT PULL] Networking deletions for 7.1
From: Jason Xing @ 2026-04-24 2:39 UTC (permalink / raw)
To: Roman Gushchin
Cc: Jakub Kicinski, davem, netdev, linux-kernel, Paolo Abeni,
Linus Torvalds
In-Reply-To: <7ia4se8lb0vf.fsf@castle.c.googlers.com>
On Fri, Apr 24, 2026 at 9:31 AM Roman Gushchin <roman.gushchin@linux.dev> wrote:
>
>
> 2c on Sashiko:
>
> 1) I'm working on an infrastructure to separate pre-existing issues from
> new issues. My current thinking is to stop reporting these issues with
> reviews of new patches and instead put them into some database and give
> maintainers access to it. Sashiko will automatically deduplicate issues
> and index them by the source file/subsystem. Hopefully it will mean that
> maintainers will see only a limited number of issues in source files
> they support. But I have yet to see how it works in practice.
>
> But I'm somewhat concerned that this way many of these issues will
> remain there forever and by reporting them with new material we actually
> have better chances to get them fixes. Maybe it should be configurable
> per-subsystem. I'm very open for ideas here.
I reckon reporting pre-existing bugs is very meaningful, especially
for maintainers/reviewers/key developers who are trying so hard to
improve this area. But it's a burden for a one-time developer to
understand and fix those pre-existing buggy behavior.
Syzbot has its standalone place for people who are willing to keep
track of known reports: https://syzkaller.appspot.com/upstream/s/net.
Maybe we could do something quite similar?
Thanks,
Jason
>
> 2) Re false positives vs finding more bugs I had the same experience.
> It's easy to tweak it to be more conservative or creative, but it comes
> at a price. It seems like the real answer is simple a better model. We
> saw a big improvement internally switching from Gemini Pro 3.0 to 3.1.
>
> Thanks
>
^ permalink raw reply
* [PATCH v2] ice: wait for reset completion in ice_resume()
From: Aaron Ma @ 2026-04-24 3:03 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
Cc: Akeem G Abodunrin, Jesse Brandeburg, intel-wired-lan, Kohei Enju
ice_resume() schedules an asynchronous PF reset and returns
immediately. The reset runs later in ice_service_task(). If
userspace tries to bring up the net device before the reset
finishes, ice_open() fails with -EBUSY:
ice_resume()
ice_schedule_reset() # sets ICE_PFR_REQ, returns
...
ice_open()
ice_is_reset_in_progress() # ICE_PFR_REQ still set, -EBUSY
...
ice_service_task()
ice_do_reset()
ice_rebuild() # clears ICE_PFR_REQ, too late
Reproduced on E800 series NICs during suspend/resume with irdma
enabled, where the aux device probe widens the race window.
Wait for the reset to complete before returning from ice_resume().
Fixes: 769c500dcc1e ("ice: Add advanced power mgmt for WoL")
Cc: stable@vger.kernel.org
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
v2: reword comment to clarify best-effort semantics (Kohei Enju)
drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 5f92377d4dfc2..a81eb21ea87c1 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5635,6 +5635,15 @@ static int ice_resume(struct device *dev)
/* Restart the service task */
mod_timer(&pf->serv_tmr, round_jiffies(jiffies + pf->serv_tmr_period));
+ /* Best-effort wait for the scheduled reset to finish so that the
+ * device is operational before returning. Without this, userspace
+ * (e.g. NetworkManager) may try to open the net device while the
+ * asynchronous reset is still in progress, hitting -EBUSY.
+ */
+ ret = ice_wait_for_reset(pf, 10 * HZ);
+ if (ret)
+ dev_err(dev, "Wait for reset failed during resume: %d\n", ret);
+
return 0;
}
--
2.43.0
^ permalink raw reply related
* Re: [GIT PULL] Networking deletions for 7.1
From: Jakub Kicinski @ 2026-04-24 3:10 UTC (permalink / raw)
To: Roman Gushchin; +Cc: davem, netdev, linux-kernel, Paolo Abeni, Linus Torvalds
In-Reply-To: <7ia4se8lb0vf.fsf@castle.c.googlers.com>
On Fri, 24 Apr 2026 01:31:00 +0000 Roman Gushchin wrote:
> 2c on Sashiko:
Did I say Sashiko 3 times? ;)
FTR - thanks a lot for the awesome work on that project!
> 1) I'm working on an infrastructure to separate pre-existing issues from
> new issues. My current thinking is to stop reporting these issues with
> reviews of new patches and instead put them into some database and give
> maintainers access to it. Sashiko will automatically deduplicate issues
> and index them by the source file/subsystem. Hopefully it will mean that
> maintainers will see only a limited number of issues in source files
> they support. But I have yet to see how it works in practice.
>
> But I'm somewhat concerned that this way many of these issues will
> remain there forever and by reporting them with new material we actually
> have better chances to get them fixes. Maybe it should be configurable
> per-subsystem. I'm very open for ideas here.
After a couple of hours of glasswing-inspired hacking today I wonder
if we can close the loop on patch generation without blowing the budget.
Or maybe a better question would be - how many of the issues are simple
enough for the LLM to pop out a patch for, once it has all the context
from the review.
> 2) Re false positives vs finding more bugs I had the same experience.
> It's easy to tweak it to be more conservative or creative, but it comes
> at a price. It seems like the real answer is simple a better model. We
> saw a big improvement internally switching from Gemini Pro 3.0 to 3.1.
Makes sense. What didn't really land for me until now is that at least
for now different models are better at catching different types of bugs.
Off the top of my head, Gemini does short work of misuses of DMA API
without much context. Claude/codex/whatever the Meta one is called
externally - none of them catch those.
^ permalink raw reply
* [PATCH iwl-next] libie: log more info when virtchnl fails
From: Li Li @ 2026-04-24 3:15 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, David S. Miller, Jakub Kicinski,
Eric Dumazet, intel-wired-lan
Cc: netdev, linux-kernel, David Decotigny, Anjali Singhai,
Sridhar Samudrala, Brian Vazquez, Li Li, emil.s.tantilov
Virtchnl failures can be hard to debug without logs. Logging the details
of virtchnl transactions can be useful for debugging virtchnl-related
issues.
Tested: Built and booted on a test machine.
Signed-off-by: Li Li <boolli@google.com>
---
drivers/net/ethernet/intel/libie/controlq.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
index ebc05355e39d..7eaa77413621 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -762,6 +762,16 @@ libie_ctlq_xn_process_recv(struct libie_ctlq_xn_recv_params *params,
status = ctlq_msg->chnl_retval ? -EFAULT : 0;
xn = &xnm->ring[xn_index];
+
+ if (ctlq_msg->chnl_retval) {
+ dev_err_ratelimited(
+ params->ctlq->dev,
+ "Non-zero virtchnl ret val (msg op: %u, ret val: %u, msg_cookie: %u, data_len: %u); xn op: %u, id: %u, cookie: %u\n",
+ ctlq_msg->chnl_opcode, ctlq_msg->chnl_retval,
+ msg_cookie, ctlq_msg->data_len, xn->virtchnl_opcode,
+ xn->index, xn->cookie);
+ }
+
if (ctlq_msg->chnl_opcode != xn->virtchnl_opcode ||
msg_cookie != xn->cookie)
return false;
@@ -1011,6 +1021,11 @@ int libie_ctlq_xn_send(struct libie_ctlq_xn_send_params *params)
params->recv_mem = xn->recv_mem;
break;
default:
+ dev_notice_ratelimited(
+ params->ctlq->dev,
+ "Transaction failed (op %u, xn state: %d, id: %u, cookie: %u, size: %zu)\n",
+ params->chnl_opcode, xn->state, xn->index, xn->cookie,
+ xn->recv_mem.iov_len);
ret = -EBADMSG;
break;
}
--
2.54.0.rc2.544.gc7ae2d5bb8-goog
^ permalink raw reply related
* Re: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
From: syzbot @ 2026-04-24 3:16 UTC (permalink / raw)
To: 25181214217
Cc: 25181214217, davem, dsahern, edumazet, horms, kuba, linux-kernel,
netdev, pabeni, sd, willemdebruijn.kernel
In-Reply-To: <2ebe2714.7628.19dbd7d2b62.Coremail.25181214217@stu.xidian.edu.cn>
> Hi,
>
> Thank you so much for the review and for pointing me to the correct Fixes tag!
>
> You hit the nail on the head regarding `__ip6_append_data()`. After re-evaluating the code path based on your question, I realize my assumption in v2 was incorrect. `__ip6_append_data()` does indeed guarantee that an skb is queued upon success, making the `skb == NULL` path dead code in this context.
>
> I traced the `failslab` memory leak back to its true origin: the lockless fast path wrapper `ip6_make_skb()`.
>
> Sabrina previously noted that `ip6_setup_cork()` failures correctly release the dst. That is absolutely true for the slow path, where `udp_v6_flush_pending_frames()` eventually handles the cleanup.
>
> However, in the fast path, `ip6_make_skb()` calls `ip6_setup_cork()`. Inside `ip6_setup_cork()`, `cork->base.dst` is assigned early. If a subsequent memory allocation fails (e.g., `v6_cork->opt = kzalloc(...)` failing due to failslab), it returns an error. `ip6_make_skb()` then directly returns `ERR_PTR(err)` WITHOUT calling `ip6_cork_release(cork)`.
>
> Since `udpv6_sendmsg()` assumes the `dst` reference is stolen by `ip6_make_skb()` and unconditionally jumps to `out_no_dst`, the `dst` is completely leaked.
>
> The fix is simply to add `ip6_cork_release(cork)` in the `ip6_setup_cork()` error path inside `ip6_make_skb()`.
>
> I will submit a v3 patch shortly addressing this true root cause and using your suggested Fixes tag. Thank you again for steering me in the exact right direction!
>
> Best regards,
> Mingyu Wang
>
>
>> -----原始邮件-----
>> 发件人: "Willem de Bruijn" <willemdebruijn.kernel@gmail.com>
>> 发送时间:2026-04-23 22:59:45 (星期四)
>> 收件人: "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, willemdebruijn.kernel@gmail.com, davem@davemloft.net, dsahern@kernel.org, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com
>> 抄送: sd@queasysnail.net, horms@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
>> 主题: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
>>
>> Mingyu Wang wrote:
>> > During fuzzing with failslab enabled, a memory leak was observed in the
>> > IPv6 UDP send path.
>> >
>> > The root cause resides in __ip6_make_skb(). In extremely rare cases
>> > (such as fault injection or specific empty payload conditions),
>>
>> Can you elaborate on this? Which fault injection lets
>> __ip6_append_data succeed without writing data?
>>
>> > __ip6_append_data() may succeed but leave the socket's write queue
>> > empty.
>> >
>> > When __ip6_make_skb() is subsequently called, __skb_dequeue(queue)
>> > returns NULL. The previous logic handled this by executing a 'goto out;',
>> > which completely bypassed the call to ip6_cork_release(cork).
>> >
>> > Since the 'cork' structure actively holds a reference to the routing
>> > entry (dst_entry) and potentially other allocated options, skipping
>> > the release cleanly leaks these resources.
>> >
>> > Fix this by introducing an 'out_cork_release' label and jumping to it
>> > when skb is NULL, ensuring the cork state is always properly cleaned up.
>> > The now-unused 'out' label is also removed to prevent compiler warnings.
>> >
>> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>
>> I think this is
>>
>> Fixes: 6422398c2ab0 ("ipv6: introduce ipv6_make_skb")
>>
>> > Reported-by: syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
>> > Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
I see the command but can't find the corresponding bug.
The email is sent to syzbot+HASH@syzkaller.appspotmail.com address
but the HASH does not correspond to any known bug.
Please double check the address.
^ permalink raw reply
* Re: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
From: 王明煜 @ 2026-04-24 3:16 UTC (permalink / raw)
To: Willem de Bruijn
Cc: davem, dsahern, edumazet, kuba, pabeni, sd, horms, netdev,
linux-kernel, syzbot+e5d6936b9f4545fd88ab
In-Reply-To: <willemdebruijn.kernel.28284ffff84dc@gmail.com>
Hi,
Thank you so much for the review and for pointing me to the correct Fixes tag!
You hit the nail on the head regarding `__ip6_append_data()`. After re-evaluating the code path based on your question, I realize my assumption in v2 was incorrect. `__ip6_append_data()` does indeed guarantee that an skb is queued upon success, making the `skb == NULL` path dead code in this context.
I traced the `failslab` memory leak back to its true origin: the lockless fast path wrapper `ip6_make_skb()`.
Sabrina previously noted that `ip6_setup_cork()` failures correctly release the dst. That is absolutely true for the slow path, where `udp_v6_flush_pending_frames()` eventually handles the cleanup.
However, in the fast path, `ip6_make_skb()` calls `ip6_setup_cork()`. Inside `ip6_setup_cork()`, `cork->base.dst` is assigned early. If a subsequent memory allocation fails (e.g., `v6_cork->opt = kzalloc(...)` failing due to failslab), it returns an error. `ip6_make_skb()` then directly returns `ERR_PTR(err)` WITHOUT calling `ip6_cork_release(cork)`.
Since `udpv6_sendmsg()` assumes the `dst` reference is stolen by `ip6_make_skb()` and unconditionally jumps to `out_no_dst`, the `dst` is completely leaked.
The fix is simply to add `ip6_cork_release(cork)` in the `ip6_setup_cork()` error path inside `ip6_make_skb()`.
I will submit a v3 patch shortly addressing this true root cause and using your suggested Fixes tag. Thank you again for steering me in the exact right direction!
Best regards,
Mingyu Wang
> -----原始邮件-----
> 发件人: "Willem de Bruijn" <willemdebruijn.kernel@gmail.com>
> 发送时间:2026-04-23 22:59:45 (星期四)
> 收件人: "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, willemdebruijn.kernel@gmail.com, davem@davemloft.net, dsahern@kernel.org, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com
> 抄送: sd@queasysnail.net, horms@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
> 主题: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
>
> Mingyu Wang wrote:
> > During fuzzing with failslab enabled, a memory leak was observed in the
> > IPv6 UDP send path.
> >
> > The root cause resides in __ip6_make_skb(). In extremely rare cases
> > (such as fault injection or specific empty payload conditions),
>
> Can you elaborate on this? Which fault injection lets
> __ip6_append_data succeed without writing data?
>
> > __ip6_append_data() may succeed but leave the socket's write queue
> > empty.
> >
> > When __ip6_make_skb() is subsequently called, __skb_dequeue(queue)
> > returns NULL. The previous logic handled this by executing a 'goto out;',
> > which completely bypassed the call to ip6_cork_release(cork).
> >
> > Since the 'cork' structure actively holds a reference to the routing
> > entry (dst_entry) and potentially other allocated options, skipping
> > the release cleanly leaks these resources.
> >
> > Fix this by introducing an 'out_cork_release' label and jumping to it
> > when skb is NULL, ensuring the cork state is always properly cleaned up.
> > The now-unused 'out' label is also removed to prevent compiler warnings.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>
> I think this is
>
> Fixes: 6422398c2ab0 ("ipv6: introduce ipv6_make_skb")
>
> > Reported-by: syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
> > Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
^ permalink raw reply
* Re: Re: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
From: 王明煜 @ 2026-04-24 3:26 UTC (permalink / raw)
To: Willem de Bruijn
Cc: davem, dsahern, edumazet, kuba, pabeni, sd, horms, netdev,
linux-kernel, syzbot+e5d6936b9f4545fd88ab
In-Reply-To: <2ebe2714.7628.19dbd7d2b62.Coremail.25181214217@stu.xidian.edu.cn>
Hi Sabrina and Jakub,
Before sending out the v3 patch, I synced my tree to the latest mainline and checked the current state of `ip6_make_skb()`.
It turns out that the missing `ip6_cork_release(cork)` in the error path was already naturally resolved by Eric Dumazet's recent refactoring commit:
b409a7f7176b ("ipv6: colocate inet6_cork in inet_cork_full")
With Eric's changes, the error handling now correctly calls `ip6_cork_release(cork)` if `ip6_setup_cork()` fails, meaning the memory leak is no longer present in the latest tree.
Please disregard my v1 and v2 patches. I am also telling syzbot to close this report based on Eric's commit.
Thank you all again for your time, the deep code review, and for guiding me to find the true root cause. I learned a huge amount from this discussion!
#syz fix: ipv6: colocate inet6_cork in inet_cork_full
Best regards,
Mingyu Wang
2026-04-24 11:16:30 "王明煜" <25181214217@stu.xidian.edu.cn> 写道：
> Hi,
>
> Thank you so much for the review and for pointing me to the correct Fixes tag!
>
> You hit the nail on the head regarding `__ip6_append_data()`. After re-evaluating the code path based on your question, I realize my assumption in v2 was incorrect. `__ip6_append_data()` does indeed guarantee that an skb is queued upon success, making the `skb == NULL` path dead code in this context.
>
> I traced the `failslab` memory leak back to its true origin: the lockless fast path wrapper `ip6_make_skb()`.
>
> Sabrina previously noted that `ip6_setup_cork()` failures correctly release the dst. That is absolutely true for the slow path, where `udp_v6_flush_pending_frames()` eventually handles the cleanup.
>
> However, in the fast path, `ip6_make_skb()` calls `ip6_setup_cork()`. Inside `ip6_setup_cork()`, `cork->base.dst` is assigned early. If a subsequent memory allocation fails (e.g., `v6_cork->opt = kzalloc(...)` failing due to failslab), it returns an error. `ip6_make_skb()` then directly returns `ERR_PTR(err)` WITHOUT calling `ip6_cork_release(cork)`.
>
> Since `udpv6_sendmsg()` assumes the `dst` reference is stolen by `ip6_make_skb()` and unconditionally jumps to `out_no_dst`, the `dst` is completely leaked.
>
> The fix is simply to add `ip6_cork_release(cork)` in the `ip6_setup_cork()` error path inside `ip6_make_skb()`.
>
> I will submit a v3 patch shortly addressing this true root cause and using your suggested Fixes tag. Thank you again for steering me in the exact right direction!
>
> Best regards,
> Mingyu Wang
>
>
> > -----原始邮件-----
> > 发件人: "Willem de Bruijn" <willemdebruijn.kernel@gmail.com>
> > 发送时间:2026-04-23 22:59:45 (星期四)
> > 收件人: "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, willemdebruijn.kernel@gmail.com, davem@davemloft.net, dsahern@kernel.org, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com
> > 抄送: sd@queasysnail.net, horms@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
> > 主题: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
> >
> > Mingyu Wang wrote:
> > > During fuzzing with failslab enabled, a memory leak was observed in the
> > > IPv6 UDP send path.
> > >
> > > The root cause resides in __ip6_make_skb(). In extremely rare cases
> > > (such as fault injection or specific empty payload conditions),
> >
> > Can you elaborate on this? Which fault injection lets
> > __ip6_append_data succeed without writing data?
> >
> > > __ip6_append_data() may succeed but leave the socket's write queue
> > > empty.
> > >
> > > When __ip6_make_skb() is subsequently called, __skb_dequeue(queue)
> > > returns NULL. The previous logic handled this by executing a 'goto out;',
> > > which completely bypassed the call to ip6_cork_release(cork).
> > >
> > > Since the 'cork' structure actively holds a reference to the routing
> > > entry (dst_entry) and potentially other allocated options, skipping
> > > the release cleanly leaks these resources.
> > >
> > > Fix this by introducing an 'out_cork_release' label and jumping to it
> > > when skb is NULL, ensuring the cork state is always properly cleaned up.
> > > The now-unused 'out' label is also removed to prevent compiler warnings.
> > >
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >
> > I think this is
> >
> > Fixes: 6422398c2ab0 ("ipv6: introduce ipv6_make_skb")
> >
> > > Reported-by: syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
> > > Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
^ permalink raw reply
* Re: Re: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
From: syzbot @ 2026-04-24 3:27 UTC (permalink / raw)
To: 25181214217
Cc: 25181214217, davem, dsahern, edumazet, horms, kuba, linux-kernel,
netdev, pabeni, sd, willemdebruijn.kernel
In-Reply-To: <5eabb1a1.7642.19dbd869da3.Coremail.25181214217@stu.xidian.edu.cn>
> Hi Sabrina and Jakub,
>
> Before sending out the v3 patch, I synced my tree to the latest mainline and checked the current state of `ip6_make_skb()`.
>
> It turns out that the missing `ip6_cork_release(cork)` in the error path was already naturally resolved by Eric Dumazet's recent refactoring commit:
> b409a7f7176b ("ipv6: colocate inet6_cork in inet_cork_full")
>
> With Eric's changes, the error handling now correctly calls `ip6_cork_release(cork)` if `ip6_setup_cork()` fails, meaning the memory leak is no longer present in the latest tree.
>
> Please disregard my v1 and v2 patches. I am also telling syzbot to close this report based on Eric's commit.
>
> Thank you all again for your time, the deep code review, and for guiding me to find the true root cause. I learned a huge amount from this discussion!
>
> #syz fix: ipv6: colocate inet6_cork in inet_cork_full
I see the command but can't find the corresponding bug.
The email is sent to syzbot+HASH@syzkaller.appspotmail.com address
but the HASH does not correspond to any known bug.
Please double check the address.
>
> Best regards,
> Mingyu Wang
>
> 2026-04-24 11:16:30 "王明煜" <25181214217@stu.xidian.edu.cn> 写道：
>> Hi,
>>
>> Thank you so much for the review and for pointing me to the correct Fixes tag!
>>
>> You hit the nail on the head regarding `__ip6_append_data()`. After re-evaluating the code path based on your question, I realize my assumption in v2 was incorrect. `__ip6_append_data()` does indeed guarantee that an skb is queued upon success, making the `skb == NULL` path dead code in this context.
>>
>> I traced the `failslab` memory leak back to its true origin: the lockless fast path wrapper `ip6_make_skb()`.
>>
>> Sabrina previously noted that `ip6_setup_cork()` failures correctly release the dst. That is absolutely true for the slow path, where `udp_v6_flush_pending_frames()` eventually handles the cleanup.
>>
>> However, in the fast path, `ip6_make_skb()` calls `ip6_setup_cork()`. Inside `ip6_setup_cork()`, `cork->base.dst` is assigned early. If a subsequent memory allocation fails (e.g., `v6_cork->opt = kzalloc(...)` failing due to failslab), it returns an error. `ip6_make_skb()` then directly returns `ERR_PTR(err)` WITHOUT calling `ip6_cork_release(cork)`.
>>
>> Since `udpv6_sendmsg()` assumes the `dst` reference is stolen by `ip6_make_skb()` and unconditionally jumps to `out_no_dst`, the `dst` is completely leaked.
>>
>> The fix is simply to add `ip6_cork_release(cork)` in the `ip6_setup_cork()` error path inside `ip6_make_skb()`.
>>
>> I will submit a v3 patch shortly addressing this true root cause and using your suggested Fixes tag. Thank you again for steering me in the exact right direction!
>>
>> Best regards,
>> Mingyu Wang
>>
>>
>> > -----原始邮件-----
>> > 发件人: "Willem de Bruijn" <willemdebruijn.kernel@gmail.com>
>> > 发送时间:2026-04-23 22:59:45 (星期四)
>> > 收件人: "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, willemdebruijn.kernel@gmail.com, davem@davemloft.net, dsahern@kernel.org, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com
>> > 抄送: sd@queasysnail.net, horms@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
>> > 主题: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
>> >
>> > Mingyu Wang wrote:
>> > > During fuzzing with failslab enabled, a memory leak was observed in the
>> > > IPv6 UDP send path.
>> > >
>> > > The root cause resides in __ip6_make_skb(). In extremely rare cases
>> > > (such as fault injection or specific empty payload conditions),
>> >
>> > Can you elaborate on this? Which fault injection lets
>> > __ip6_append_data succeed without writing data?
>> >
>> > > __ip6_append_data() may succeed but leave the socket's write queue
>> > > empty.
>> > >
>> > > When __ip6_make_skb() is subsequently called, __skb_dequeue(queue)
>> > > returns NULL. The previous logic handled this by executing a 'goto out;',
>> > > which completely bypassed the call to ip6_cork_release(cork).
>> > >
>> > > Since the 'cork' structure actively holds a reference to the routing
>> > > entry (dst_entry) and potentially other allocated options, skipping
>> > > the release cleanly leaks these resources.
>> > >
>> > > Fix this by introducing an 'out_cork_release' label and jumping to it
>> > > when skb is NULL, ensuring the cork state is always properly cleaned up.
>> > > The now-unused 'out' label is also removed to prevent compiler warnings.
>> > >
>> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> >
>> > I think this is
>> >
>> > Fixes: 6422398c2ab0 ("ipv6: introduce ipv6_make_skb")
>> >
>> > > Reported-by: syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
>> > > Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
^ permalink raw reply
* Re: [PATCH net] net: mana: hardening: Validate SHM offset from BAR0 register to prevent crash due to alignment fault
From: Dipayaan Roy @ 2026-04-24 3:28 UTC (permalink / raw)
To: Andrew Lunn
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, leon, longli, kotaranov, horms, shradhagupta,
ssengar, ernis, shirazsaleem, linux-hyperv, netdev, linux-kernel,
linux-rdma, stephen, jacob.e.keller, dipayanroy, leitao, kees,
john.fastabend, hawk, bpf, daniel, ast, sdf, yury.norov
In-Reply-To: <7c4dbe89-9b51-45d6-ae89-39d4183e66b1@lunn.ch>
On Thu, Apr 23, 2026 at 09:44:04PM +0200, Andrew Lunn wrote:
> On Thu, Apr 23, 2026 at 12:14:16PM -0700, Dipayaan Roy wrote:
> > On Thu, Apr 23, 2026 at 06:37:04PM +0200, Andrew Lunn wrote:
> > > > The root cause is in mana_gd_init_vf_regs(), which computes:
> > > >
> > > > gc->shm_base = gc->bar0_va + mana_gd_r64(gc, GDMA_REG_SHM_OFFSET);
> > > >
> > > > without validating the offset read from hardware. If the register
> > > > returns a garbage value that is neither within bar 0 bounds nor aligned
> > > > to the 4-byte granularity, thus causing the alignment fault.
> > >
> > > Is GDMA_REG_SHM_OFFSET special?
> > Hi Andrew,
> > GDMA_REG_SHM_OFFSET is not special. It was simply the only register
> > read that had no validation at all. The other two registers
> > (GDMA_REG_DB_PAGE_SIZE, GDMA_REG_DB_PAGE_OFFSET) already have checks
> > in place.
>
> I must be missing something:
>
> grep page_size *
>
> gdma_main.c: gc->db_page_size = mana_gd_r32(gc, GDMA_PF_REG_DB_PAGE_SIZE) & 0xFFFF;
> gdma_main.c: gc->db_page_size = mana_gd_r32(gc, GDMA_REG_DB_PAGE_SIZE) & 0xFFFF;
> gdma_main.c: void __iomem *addr = gc->db_page_base + gc->db_page_size * db_index;
>
Hi Andrew,
There are 2 upstream commits regarding these, I think you missed
them please check once:
commit fb4b4a05aeeb8b0f253c5ddce21f4635dadc9550
Author: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Date: Wed Mar 25 11:04:17 2026 -0700
net: mana: Use at least SZ_4K in doorbell ID range check
commit 89fe91c65992a37863241e35aec151210efc53ce
Author: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Date: Fri Mar 6 13:12:06 2026 -0800
net: mana: hardening: Validate doorbell ID from GDMA_REGISTER_DEVICE response
> So if GDMA_REG_DB_PAGE_SIZE returns garbage, it is at least masked,
> but it is still a random number.
>
> mana_gd_ring_doorbell() takes this random number, multiples by
> db_index, adds, gc->db_page_base and then does:
>
> writeq(e.as_uint64, addr);
>
> So you write to a random address.
>
> I don't see any sanity checks here. Cannot you check that db_page_size
> is at least one of the expected page sizes?
As mentioned above checks are already present in this commit: 89fe91c65992a37863241e35aec151210efc53ce
>
> Andrew
Regards
Dipayaan Roy
^ permalink raw reply
* [PATCH v2 0/2] sungem: PHY initialization fixes and style cleanup
From: Joel @ 2026-04-24 3:33 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, linux-kernel, Joel
This series splits the previous monolithic patch per feedback from Andrew Lunn.
Patch 1 cleans up pointer formatting and indentation to align with kernel coding standards across the file.
Patch 2 implements hardware validation for the BCM5411 PHY initialization and adds error reporting to the core gem_init_phy logic
Joel (2):
sungem: fix pointer formatting and indentation
sungem: validate BCM5411 PHY register reads
drivers/net/ethernet/sun/sungem.c | 8 +++--
drivers/net/sungem_phy.c | 60 +++++++++++++++++--------------
2 files changed, 39 insertions(+), 29 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/2] sungem: fix pointer formatting and indentation
From: Joel @ 2026-04-24 3:33 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, linux-kernel, Joel
In-Reply-To: <20260424033348.1917-1-joelcrouch@gmail.com>
Align with Linux Kernel Coding Style by placing the asterisk with the variable name and correcting indentation in bcm54xx_read_link.
---
drivers/net/sungem_phy.c | 53 ++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/net/sungem_phy.c b/drivers/net/sungem_phy.c
index c10198d44..44cbe1938 100644
--- a/drivers/net/sungem_phy.c
+++ b/drivers/net/sungem_phy.c
@@ -44,27 +44,27 @@ static const int phy_BCM5400_link_table[8][3] = {
{ 1, 0, 1 }, /* 1000BT */
};
-static inline int __sungem_phy_read(struct mii_phy* phy, int id, int reg)
+static inline int __sungem_phy_read(struct mii_phy *phy, int id, int reg)
{
return phy->mdio_read(phy->dev, id, reg);
}
-static inline void __sungem_phy_write(struct mii_phy* phy, int id, int reg, int val)
+static inline void __sungem_phy_write(struct mii_phy *phy, int id, int reg, int val)
{
phy->mdio_write(phy->dev, id, reg, val);
}
-static inline int sungem_phy_read(struct mii_phy* phy, int reg)
+static inline int sungem_phy_read(struct mii_phy *phy, int reg)
{
return phy->mdio_read(phy->dev, phy->mii_id, reg);
}
-static inline void sungem_phy_write(struct mii_phy* phy, int reg, int val)
+static inline void sungem_phy_write(struct mii_phy *phy, int reg, int val)
{
phy->mdio_write(phy->dev, phy->mii_id, reg, val);
}
-static int reset_one_mii_phy(struct mii_phy* phy, int phy_id)
+static int reset_one_mii_phy(struct mii_phy *phy, int phy_id)
{
u16 val;
int limit = 10000;
@@ -88,7 +88,7 @@ static int reset_one_mii_phy(struct mii_phy* phy, int phy_id)
return limit <= 0;
}
-static int bcm5201_init(struct mii_phy* phy)
+static int bcm5201_init(struct mii_phy *phy)
{
u16 data;
@@ -101,7 +101,7 @@ static int bcm5201_init(struct mii_phy* phy)
return 0;
}
-static int bcm5201_suspend(struct mii_phy* phy)
+static int bcm5201_suspend(struct mii_phy *phy)
{
sungem_phy_write(phy, MII_BCM5201_INTERRUPT, 0);
sungem_phy_write(phy, MII_BCM5201_MULTIPHY, MII_BCM5201_MULTIPHY_SUPERISOLATE);
@@ -109,7 +109,7 @@ static int bcm5201_suspend(struct mii_phy* phy)
return 0;
}
-static int bcm5221_init(struct mii_phy* phy)
+static int bcm5221_init(struct mii_phy *phy)
{
u16 data;
@@ -132,7 +132,7 @@ static int bcm5221_init(struct mii_phy* phy)
return 0;
}
-static int bcm5221_suspend(struct mii_phy* phy)
+static int bcm5221_suspend(struct mii_phy *phy)
{
u16 data;
@@ -147,7 +147,7 @@ static int bcm5221_suspend(struct mii_phy* phy)
return 0;
}
-static int bcm5241_init(struct mii_phy* phy)
+static int bcm5241_init(struct mii_phy *phy)
{
u16 data;
@@ -170,7 +170,7 @@ static int bcm5241_init(struct mii_phy* phy)
return 0;
}
-static int bcm5241_suspend(struct mii_phy* phy)
+static int bcm5241_suspend(struct mii_phy *phy)
{
u16 data;
@@ -185,7 +185,7 @@ static int bcm5241_suspend(struct mii_phy* phy)
return 0;
}
-static int bcm5400_init(struct mii_phy* phy)
+static int bcm5400_init(struct mii_phy *phy)
{
u16 data;
@@ -214,7 +214,7 @@ static int bcm5400_init(struct mii_phy* phy)
return 0;
}
-static int bcm5400_suspend(struct mii_phy* phy)
+static int bcm5400_suspend(struct mii_phy *phy)
{
#if 0 /* Commented out in Darwin... someone has those dawn docs ? */
sungem_phy_write(phy, MII_BMCR, BMCR_PDOWN);
@@ -222,7 +222,7 @@ static int bcm5400_suspend(struct mii_phy* phy)
return 0;
}
-static int bcm5401_init(struct mii_phy* phy)
+static int bcm5401_init(struct mii_phy *phy)
{
u16 data;
int rev;
@@ -270,7 +270,7 @@ static int bcm5401_init(struct mii_phy* phy)
return 0;
}
-static int bcm5401_suspend(struct mii_phy* phy)
+static int bcm5401_suspend(struct mii_phy *phy)
{
#if 0 /* Commented out in Darwin... someone has those dawn docs ? */
sungem_phy_write(phy, MII_BMCR, BMCR_PDOWN);
@@ -278,8 +278,9 @@ static int bcm5401_suspend(struct mii_phy* phy)
return 0;
}
-static int bcm5411_init(struct mii_phy* phy)
+static int bcm5411_init(struct mii_phy *phy)
{
+ int val;
u16 data;
/* Here's some more Apple black magic to setup
@@ -408,14 +409,14 @@ static int genmii_read_link(struct mii_phy *phy)
return 0;
}
-static int generic_suspend(struct mii_phy* phy)
+static int generic_suspend(struct mii_phy *phy)
{
sungem_phy_write(phy, MII_BMCR, BMCR_PDOWN);
return 0;
}
-static int bcm5421_init(struct mii_phy* phy)
+static int bcm5421_init(struct mii_phy *phy)
{
u16 data;
unsigned int id;
@@ -548,7 +549,7 @@ static int bcm54xx_read_link(struct mii_phy *phy)
u16 val;
if (phy->autoneg) {
- val = sungem_phy_read(phy, MII_BCM5400_AUXSTATUS);
+ val = sungem_phy_read(phy, MII_BCM5400_AUXSTATUS);
link_mode = ((val & MII_BCM5400_AUXSTATUS_LINKMODE_MASK) >>
MII_BCM5400_AUXSTATUS_LINKMODE_SHIFT);
phy->duplex = phy_BCM5400_link_table[link_mode][0] ?
@@ -568,7 +569,7 @@ static int bcm54xx_read_link(struct mii_phy *phy)
return 0;
}
-static int marvell88e1111_init(struct mii_phy* phy)
+static int marvell88e1111_init(struct mii_phy *phy)
{
u16 rev;
@@ -592,7 +593,7 @@ static int marvell88e1111_init(struct mii_phy* phy)
#define BCM5421_MODE_MASK (1 << 5)
-static int bcm5421_poll_link(struct mii_phy* phy)
+static int bcm5421_poll_link(struct mii_phy *phy)
{
u32 phy_reg;
int mode;
@@ -616,7 +617,7 @@ static int bcm5421_poll_link(struct mii_phy* phy)
return 1;
}
-static int bcm5421_read_link(struct mii_phy* phy)
+static int bcm5421_read_link(struct mii_phy *phy)
{
u32 phy_reg;
int mode;
@@ -644,7 +645,7 @@ static int bcm5421_read_link(struct mii_phy* phy)
return 0;
}
-static int bcm5421_enable_fiber(struct mii_phy* phy, int autoneg)
+static int bcm5421_enable_fiber(struct mii_phy *phy, int autoneg)
{
/* enable fiber mode */
sungem_phy_write(phy, MII_NCONFIG, 0x9020);
@@ -665,7 +666,7 @@ static int bcm5421_enable_fiber(struct mii_phy* phy, int autoneg)
#define BCM5461_FIBER_LINK (1 << 2)
#define BCM5461_MODE_MASK (3 << 1)
-static int bcm5461_poll_link(struct mii_phy* phy)
+static int bcm5461_poll_link(struct mii_phy *phy)
{
u32 phy_reg;
int mode;
@@ -691,7 +692,7 @@ static int bcm5461_poll_link(struct mii_phy* phy)
#define BCM5461_FIBER_DUPLEX (1 << 3)
-static int bcm5461_read_link(struct mii_phy* phy)
+static int bcm5461_read_link(struct mii_phy *phy)
{
u32 phy_reg;
int mode;
@@ -720,7 +721,7 @@ static int bcm5461_read_link(struct mii_phy* phy)
return 0;
}
-static int bcm5461_enable_fiber(struct mii_phy* phy, int autoneg)
+static int bcm5461_enable_fiber(struct mii_phy *phy, int autoneg)
{
/* select fiber mode, enable 1000 base-X registers */
sungem_phy_write(phy, MII_NCONFIG, 0xfc0b);
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/2] sungem: validate BCM5411 PHY register reads
From: Joel @ 2026-04-24 3:33 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, linux-kernel, Joel
In-Reply-To: <20260424033348.1917-1-joelcrouch@gmail.com>
Check for -EIO and 0xffff during initialization to prevent silent failures. Add error logging to gem_init_phy to report failures.
---
drivers/net/ethernet/sun/sungem.c | 8 ++++++--
drivers/net/sungem_phy.c | 7 ++++++-
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
index 8e69d917d..525aba527 100644
--- a/drivers/net/ethernet/sun/sungem.c
+++ b/drivers/net/ethernet/sun/sungem.c
@@ -1707,8 +1707,12 @@ static void gem_init_phy(struct gem *gp)
sungem_phy_probe(&gp->phy_mii, gp->mii_phy_addr);
/* Init PHY */
- if (gp->phy_mii.def && gp->phy_mii.def->ops->init)
- gp->phy_mii.def->ops->init(&gp->phy_mii);
+ if (gp->phy_mii.def && gp->phy_mii.def->ops->init) {
+ int err = gp->phy_mii.def->ops->init(&gp->phy_mii);
+
+ if (err)
+ netdev_err(gp->dev, "PHY init failed: %d\n", err);
+ }
} else {
gem_pcs_reset(gp);
gem_pcs_reinit_adv(gp);
diff --git a/drivers/net/sungem_phy.c b/drivers/net/sungem_phy.c
index 44cbe1938..ec8800c93 100644
--- a/drivers/net/sungem_phy.c
+++ b/drivers/net/sungem_phy.c
@@ -296,7 +296,12 @@ static int bcm5411_init(struct mii_phy *phy)
sungem_phy_write(phy, MII_BMCR, BMCR_RESET);
sungem_phy_write(phy, MII_BMCR, 0x1340);
- data = sungem_phy_read(phy, MII_BCM5400_GB_CONTROL);
+ val = sungem_phy_read(phy, MII_BCM5400_GB_CONTROL);
+
+ if (val < 0 || val == 0xffff)
+ return -EIO;
+
+ data = (u16)val;
data |= MII_BCM5400_GB_CONTROL_FULLDUPLEXCAP;
sungem_phy_write(phy, MII_BCM5400_GB_CONTROL, data);
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox