Re: [PATCH net v2 2/2] ionic: free port_info when probe fails after the port is set up

On 10/19/2026 4:20 PM, netdev-bot+sashiko@kernel.org wrote:

[Some people who received this message don't often get email from netdev-bot+sashiko@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


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

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

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 new ionic_port_reset() call in ionic_probe()'s err_out_pci
  frees the port_info buffer with dma_free_coherent() even when the…

--

ionic: free port_info when probe fails after the port is set up

This patch adds an ionic_port_reset() call at the err_out_pci label in
ionic_probe(). The goal is to free the port_info DMA buffer that
ionic_setup_one()->ionic_port_init() allocated when probe fails later.
It follows the cleanup order already used in ionic_remove().

quoted

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 05f19489ec5c8..68b3bceedd999 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c

@@ -397,6 +397,7 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 err_out_free_irqs:
      ionic_bus_free_irq_vectors(ionic);
 err_out_pci:
+     ionic_port_reset(ionic);

[Severity: Medium]
Is it safe to free port_info here if the PORT_RESET command never
completes?

ionic_port_reset() sends the command, but the wait result does not
decide whether the free happens:

drivers/net/ethernet/pensando/ionic/ionic_main.c:ionic_port_reset() {
    ...
        if (ionic_is_fw_running(idev)) {
                mutex_lock(&ionic->dev_cmd_lock);
                ionic_dev_cmd_port_reset(idev);
                err = ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);
                mutex_unlock(&ionic->dev_cmd_lock);
        }

        dma_free_coherent(ionic->dev, idev->port_info_sz,
                          idev->port_info, idev->port_info_pa);
    ...
}

__ionic_dev_cmd_wait() can return -ETIMEDOUT while the firmware still
reports that it is running:

        if (!done && !time_before(jiffies, max_wait)) {
                ionic_dev_cmd_clean(ionic);
                dev_warn(ionic->dev, "DEVCMD %s (%d) timeout after %ld secs\n",
                         ionic_opcode_to_str(opcode), opcode, max_seconds);
                return -ETIMEDOUT;
        }

ionic_dev_cmd_port_init() passed port_info_pa to the firmware through
port_init.info_pa. The comment in ionic_get_link_ksettings() says the NIC
updates this buffer by DMA on its own.

Bus mastering, enabled by pci_set_master() in ionic_setup_one(), stays on
until pci_disable_device() in ionic_clear_pci(). That runs only after
ionic_dev_teardown().

Take a probe failure like an adminq timeout in ionic_lif_init(). The
firmware could be running but not answering dev-cmds. If it keeps writing
port status into port_info during that window, can it DMA into pages that
have already been freed and possibly reused?

Before this patch, the buffer was leaked on this path instead of freed.

The commit message says:

    the firmware gets a PORT_RESET before the buffer is freed, the same
    order as in ionic_remove().

However, the command is only sent here, and its completion is never
checked. ionic_remove() uses the same pattern, so this patch carries that
convention over to the probe failure path.

Should the free be skipped when the PORT_RESET wait fails? Alternatively,
could it be deferred until bus mastering has been disabled?

quoted

      ionic_dev_teardown(ionic);
      ionic_clear_pci(ionic);
      ionic_debugfs_del_dev(ionic);

--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261015-ionic-port-info-lifetime-v2-0-57232013fd79%40amd.com

Hi netdev's Sashiko Bot,

This finding is that this invites a change to cleanup handling in ionic that is
outside the scope of the patch; this patch does attempt to slightly clean up
things by changing a leak to a proper free while tearing down the driver after a
failure in probe, but it is valid that the device could still write into the
freed memory if that cleanup also has a failure.

I think a proper fix could be patterned after Nikhil's patch in 6443f4f20bda
("pds_core: quiesce DMA before freeing resources"), but that was applied to
net-next:
https://lore.kernel.org/all/178096501514.1726280.11716563786761485251.git-patchwork-notify@kernel.org/ (local)

- Eric