[PATCH v2 0/2] net: ena: fix resource cleanup on probe failure

[PATCH v2 0/2] net: ena: fix resource cleanup on probe failure

[Guangshuo Li]

This series fixes two resource leaks in the ena_probe() error path after
ena_device_init() has successfully initialized device resources.

Patch 1 adds the missing PHC cleanup.
Patch 2 adds the missing MMIO read request cleanup.

v2:
  - Resend the PHC and MMIO cleanup fixes as a two-patch series, as
    requested by Arthur Kiyanovski.


Guangshuo Li (2):
  net: ena: fix PHC cleanup on probe failure
  net: ena: fix MMIO read buffer leak on probe failure

 drivers/net/ethernet/amazon/ena/ena_netdev.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.43.0

[PATCH v2 1/2] net: ena: fix PHC cleanup on probe failure

[Guangshuo Li]

ena_probe() initializes the PHC as part of ena_device_init(), but the
probe failure path does not destroy it before freeing the PHC private
data.

The normal removal path calls ena_phc_destroy() through
ena_destroy_device() before ena_phc_free(). However, if probe fails
after ena_device_init() succeeds, the error path reaches ena_phc_free()
without unregistering the PTP clock or destroying the device PHC
resources.

Call ena_phc_destroy() in the probe error path before freeing the PHC
private data.

This issue was found by manual code inspection.

Fixes: e0ea34158ee8 ("net: ena: Add PHC support in the ENA driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index ea89619039d8..5f0864d16dd3 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -4122,6 +4122,7 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_device_destroy:
 	ena_com_delete_host_info(ena_dev);
 	ena_com_admin_destroy(ena_dev);
+	ena_phc_destroy(adapter);
 ena_devlink_destroy:
 	ena_devlink_free(devlink);
 err_metrics_destroy:
-- 
2.43.0

Re: [PATCH v2 1/2] net: ena: fix PHC cleanup on probe failure

[Arthur Kiyanovski]

On Mon, 21 Sep 2026 23:42:01 +0800, Guangshuo Li <lgs201920130244@gmail.com> wrote:
> ena_probe() initializes the PHC as part of ena_device_init(), but the
> probe failure path does not destroy it before freeing the PHC private
> data.
> 
> The normal removal path calls ena_phc_destroy() through
> ena_destroy_device() before ena_phc_free(). However, if probe fails
> after ena_device_init() succeeds, the error path reaches ena_phc_free()

This part:

> without unregistering the PTP clock or destroying the device PHC
> resources.

is inaccurate, see my explanation bellow.

>
>
> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index ea89619039d8..5f0864d16dd3 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -4122,6 +4122,7 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  err_device_destroy:
>  	ena_com_delete_host_info(ena_dev);
>  	ena_com_admin_destroy(ena_dev);
> +	ena_phc_destroy(adapter);
>  ena_devlink_destroy:
>  	ena_devlink_free(devlink);
>  err_metrics_destroy:

Thanks for resending as a series - the split and the ordering are what I
asked for.

But v2 doesn't address Sashiko's review of v1.

PHC is off during an initial probe. It only turns on if the user enables
it via devlink, and devlink is registered at the very end of a successful
ena_probe(). So on this error path phc_info->clock and
ena_dev->phc.virt_addr are both NULL and ena_phc_destroy() does nothing.

I still want the call - it keeps the probe unwind symmetric with
ena_destroy_device() - but it is a robustness change, not a fix. Please
drop Fixes: and Cc: stable, and say so in the commit message, for
example:

  No functional change: PHC is always disabled during an initial probe,
  so this only keeps the probe unwind symmetric.

Since net is for fixes, that means splitting after all - sorry for the
extra round trip:

 - the MMIO read buffer fix goes to net on its own, keeping Fixes: and
 - this patch goes to net-next, without Fixes: or Cc: stable

The MMIO fix has to land in net and propagate into net-next before you
post this one, otherwise you hit the same hunk conflict again.

Also please use --subject-prefix='PATCH net-next v3';
netdev/series_format is still warning that the target tree isn't
specified in the subject.

Thank you.

Cc: stable

Cc: stable

-- 
Arthur Kiyanovski <akiyano@amazon.com>

Re: [PATCH v2 1/2] net: ena: fix PHC cleanup on probe failure

[Krzysztof Kozlowski]

On 21/09/2026 21:06, Arthur Kiyanovski wrote:
> On Mon, 21 Sep 2026 23:42:01 +0800, Guangshuo Li <lgs201920130244@gmail.com> wrote:
>> ena_probe() initializes the PHC as part of ena_device_init(), but the
>> probe failure path does not destroy it before freeing the PHC private
>> data.
>>
>> The normal removal path calls ena_phc_destroy() through
>> ena_destroy_device() before ena_phc_free(). However, if probe fails
>> after ena_device_init() succeeds, the error path reaches ena_phc_free()
> 
> This part:
> 
>> without unregistering the PTP clock or destroying the device PHC
>> resources.
> 
> is inaccurate, see my explanation bellow.
> 

Don't waste your time. This is AI slop agent posting (one of 500 more of
postings where agent does not care about replies).

Best regards,
Krzysztof

Re: [PATCH v2 1/2] net: ena: fix PHC cleanup on probe failure

[Guangshuo Li]

Hi Krzysztof,

Thank you for your feedback.

On Tue, 22 Sept 2026 at 06:02, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 21/09/2026 21:06, Arthur Kiyanovski wrote:
> > On Mon, 21 Sep 2026 23:42:01 +0800, Guangshuo Li <lgs201920130244@gmail.com> wrote:
> >> ena_probe() initializes the PHC as part of ena_device_init(), but the
> >> probe failure path does not destroy it before freeing the PHC private
> >> data.
> >>
> >> The normal removal path calls ena_phc_destroy() through
> >> ena_destroy_device() before ena_phc_free(). However, if probe fails
> >> after ena_device_init() succeeds, the error path reaches ena_phc_free()
> >
> > This part:
> >
> >> without unregistering the PTP clock or destroying the device PHC
> >> resources.
> >
> > is inaccurate, see my explanation bellow.
> >
>
> Don't waste your time. This is AI slop agent posting (one of 500 more of
> postings where agent does not care about replies).
>
> Best regards,
> Krzysztof

I would like to clarify that these patches were manually reviewed and
audited by us; they were not simply generated and submitted by an LLM.
However, I understand why the recent submission pattern may have given
that impression. We sent too many patches in a short period of time,
and we also failed to respond to some discussions in a timely manner,
which made the situation look worse.

Many of the recent patches, especially the v2 revisions, are
corrections and improvements based on previous review feedback rather
than completely new untested changes. That said, we recognize that the
way we submitted them increased the burden on maintainers and
reviewers.

We apologize for the pressure this caused to the community. We will be
more careful about organizing patches by subsystem, preparing proper
patchsets, and following the kernel contribution guidelines before
sending future work.

Thank you again for pointing this out.

Best regards,
Guangshuo

Re: [PATCH v2 1/2] net: ena: fix PHC cleanup on probe failure

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The bug the commit message describes is not reachable: on
  every `ena_probe()` error path there is nothing for the newly added…

Pre-existing issues:
- [High] Use-after-free of `phc_info` (vfree()d vmalloc memory) that the
  PTP class driver still references, because `ena_phc_destroy()` ->…
- [Low] Pre-existing leak of the coherent DMA buffer allocated by
  `ena_com_mmio_reg_read_request_init()` on `ena_probe()` failure: at…

--

net: ena: fix PHC cleanup on probe failure

This adds a call to ena_phc_destroy() to the err_device_destroy label in
ena_probe(), so that the PHC is torn down before ena_phc_free() frees the
PHC private data on a probe failure after ena_device_init() succeeded.

> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> index ea89619039d89..5f0864d16dd3c 100644
> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
> @@ -4122,6 +4122,7 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  err_device_destroy:
>  	ena_com_delete_host_info(ena_dev);
>  	ena_com_admin_destroy(ena_dev);
> +	ena_phc_destroy(adapter);

[Severity: Medium]
Is there actually anything for this new call to destroy on the probe error
paths?

The commit message says:

  "if probe fails after ena_device_init() succeeds, the error path reaches
   ena_phc_free() without unregistering the PTP clock or destroying the
   device PHC resources."

Following the probe-time flow, ena_phc_init() appears to bail out before
either ena_com_phc_init() or ptp_clock_register() runs:

drivers/net/ethernet/amazon/ena/ena_phc.c:ena_phc_init() {
	/* Validate PHC feature is enabled by the kernel */
	if (!ena_phc_is_enabled(adapter)) {
		netdev_dbg(netdev, "PHC feature is not enabled by the kernel\n");
		goto err_ena_com_phc_init;
	}
	...
}

ena_phc_alloc() uses vzalloc(), so phc_info->enabled starts false, and
ena_devlink_configure_params() seeds the DRIVERINIT enable_phc value from
that same state:

drivers/net/ethernet/amazon/ena/ena_devlink.c:ena_devlink_configure_params() {
	value.vbool = ena_phc_is_enabled(adapter);
	devl_param_driverinit_value_set(devlink,
					DEVLINK_PARAM_GENERIC_ID_ENABLE_PHC,
					&value);
}

ena_device_init() reads it back through ena_devlink_params_get(), which
calls ena_phc_enable(adapter, false).  The only way to get a true value is
userspace via devlink, and devl_register() happens at the end of a
successful ena_probe(); later devlink reloads go through
ena_restore_device(), not ena_probe().

So on the error path ena_phc_is_active() is false (clock is NULL) and
ena_com_phc_destroy() returns immediately:

drivers/net/ethernet/amazon/ena/ena_com.c:ena_com_phc_destroy() {
	/* In case PHC is not supported by the device, silently exiting */
	if (!phc->virt_addr)
		return;
	...
}

If that reading is right, both halves of the added call are no-ops here.
Would it make sense to drop the Fixes: e0ea34158ee8 and
Cc: stable@vger.kernel.org tags and describe this as a consistency cleanup
instead, or to include the sequence that actually leaves a registered PTP
clock behind on a probe failure?

[Severity: High]
This isn't a bug introduced by this patch, but does ena_phc_destroy() give
the caller what the commit message assumes, namely a guaranteed unregister
before the private data is freed?

ena_phc_unregister() skips ptp_clock_unregister() whenever
ENA_FLAG_TRIGGER_RESET is set:

drivers/net/ethernet/amazon/ena/ena_phc.c:ena_phc_unregister() {
	/* During reset flow, PHC must stay registered
	 * to keep kernel's PHC index
	 */
	if (ena_phc_is_active(adapter) &&
	    !test_bit(ENA_FLAG_TRIGGER_RESET, &adapter->flags)) {
		ptp_clock_unregister(phc_info->clock);
		phc_info->clock = NULL;
	}
}

In ena_destroy_device() the flag is cleared only after ena_phc_destroy()
has already run, and ena_reset_device() (ena_netdev.h) sets that flag
lock-free from NAPI/xmit/timer contexts.

One sequence on unbind/rmmod: ena_timer_service() queues reset_task with
the flag already set, then __ena_shutoff() does

	timer_delete_sync(&adapter->timer_service);
	cancel_work_sync(&adapter->reset_task);
	...
	ena_destroy_device(adapter, true);

	ena_phc_free(adapter);

cancel_work_sync() cancels the queued work but nothing clears
ENA_FLAG_TRIGGER_RESET, so ena_phc_destroy() skips the unregister and
ena_phc_free() then vfree()s phc_info with the clock still registered.

A second sequence: a FW reset where ena_destroy_device(adapter, false)
skipped the unregister, then ena_device_init() fails in
ena_restore_device() and the err: label clears ENA_FLAG_DEVICE_RUNNING:

drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_restore_device() {
err:
	clear_bit(ENA_FLAG_DEVICE_RUNNING, &adapter->flags);
	...
}

A later unbind then hits the early return in ena_destroy_device():

	if (!test_bit(ENA_FLAG_DEVICE_RUNNING, &adapter->flags))
		return 0;

which never reaches ena_phc_destroy() at all, and ena_phc_free() frees
phc_info anyway.

In both cases /dev/ptpN stays alive with ptp_clock_info ops pointing into
freed vmalloc memory, and any clock_gettime or PTP_SYS_OFFSET on it enters

drivers/net/ethernet/amazon/ena/ena_phc.c:ena_phc_gettimex64() {
	struct ena_phc_info *phc_info =
		container_of(clock_info, struct ena_phc_info, clock_info);
	...
	spin_lock_irqsave(&phc_info->lock, flags);
	rc = ena_com_phc_get_timestamp(phc_info->adapter->ena_dev, &timestamp_nsec);
}

Can this dereference freed memory?  Should the teardown paths distinguish
reset-preserving teardown from final teardown, for example a forced
unregister variant used by ena_phc_free()/__ena_shutoff(), rather than
adding another call to the reset-conditional helper?

[Severity: Low]
This is a pre-existing issue, but does this cleanup chain leak the coherent
DMA buffer allocated by ena_com_mmio_reg_read_request_init()?

The err_device_destroy label goes ena_com_delete_host_info() ->
ena_com_admin_destroy() -> ena_phc_destroy() -> ena_devlink_free() -> ... ->
vfree(ena_dev) -> pci_disable_device() with no
ena_com_mmio_reg_read_request_destroy(), while both peer teardown sites do
call it:

drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_device_init() {
err_mmio_read_less:
	ena_com_mmio_reg_read_request_destroy(ena_dev);
	...
}

drivers/net/ethernet/amazon/ena/ena_netdev.c:ena_destroy_device() {
	ena_phc_destroy(adapter);

	ena_com_mmio_reg_read_request_destroy(ena_dev);
	...
}

so mmio_read->read_resp stays allocated and the device's MMIO_RESP_LO/HI
registers still hold the freed DMA address.

Note this is addressed by the following patch in the same series,
"net: ena: fix MMIO read buffer leak on probe failure", which inserts
ena_com_mmio_reg_read_request_destroy(ena_dev) directly after the line
added here, so no separate action looks necessary for the series as a whole.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921154202.471662-1-lgs201920130244%40gmail.com

[PATCH v2 2/2] net: ena: fix MMIO read buffer leak on probe failure

[Guangshuo Li]

ena_device_init() initializes the MMIO read mechanism with
ena_com_mmio_reg_read_request_init(), which allocates a coherent DMA
buffer for MMIO read responses.

The normal removal path releases this buffer through
ena_com_mmio_reg_read_request_destroy(). However, if ena_probe() fails
after ena_device_init() succeeds, the error path destroys the admin
resources and eventually frees ena_dev without destroying the MMIO read
request, leaving the coherent DMA buffer allocated.

Call ena_com_mmio_reg_read_request_destroy() in the probe error path
before releasing the remaining device resources.

This issue was found by manual code inspection.

Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 5f0864d16dd3..7eb6456ed0d5 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -4123,6 +4123,7 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	ena_com_delete_host_info(ena_dev);
 	ena_com_admin_destroy(ena_dev);
 	ena_phc_destroy(adapter);
+	ena_com_mmio_reg_read_request_destroy(ena_dev);
 ena_devlink_destroy:
 	ena_devlink_free(devlink);
 err_metrics_destroy:
-- 
2.43.0