All of lore.kernel.org
 help / color / mirror / Atom feed
From: Damien Le Moal <dlemoal@kernel.org>
To: Niklas Cassel <cassel@kernel.org>,
	Hans de Goede <hansg@kernel.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Jeff Garzik <jgarzik@redhat.com>
Cc: stable@vger.kernel.org, linux-ide@vger.kernel.org
Subject: Re: [PATCH v5 5/6] ata: libata: Do not leave ata_host_stop() registered when activation fails
Date: Tue, 15 Sep 2026 15:52:10 +0700	[thread overview]
Message-ID: <a478aaf0-5303-4cfe-a85a-3773f5cc9558@kernel.org> (raw)
In-Reply-To: <20260915084127.692494-13-cassel@kernel.org>

On 2026/09/15 15:41, Niklas Cassel wrote:
> ata_host_start() registers ata_host_stop() as a devres action as soon as
> it has succeeded, which hands the release of the host resources over to
> devres: ->port_stop() and ->host_stop() are then called by the driver
> core when probe() fails.
> 
> ata_host_activate() and ahci_host_activate_multi_irqs() can both fail
> after ata_host_start() has succeeded, e.g. if devm_request_irq() or
> ata_host_register() fails, and they return the error with the devres
> action still registered. A driver which releases the host resources in
> its probe() error path therefore releases them twice: once itself and
> once through ->host_stop().
> 
> All ahci-platform drivers are in that situation, e.g. ahci_probe() calls
> ahci_platform_disable_resources() while ahci_host_stop() does the same
> through devres. This gives refcount underflow warnings from the clk,
> regulator and phy cores and, for shared resources, can disable resources
> which are still in use by other devices.
> 
> Add ata_host_undo_start(), which stops the ports and drops the devres
> action without calling ->host_stop(), and call it from both activation
> helpers when they fail. Releasing the host resources on failure is then
> always left to the caller, which is what all callers having a probe()
> error path already assume.
> 
> This changes the semantics for the drivers which implement ->host_stop()
> while also completely lacking error handling for the activate host call.
> Add activate host error handling for sata_qstor and sata_fsl. For
> sata_fsl this also means that hcr_base and host_priv are now released
> when activating the host fails, which ->host_stop() did not do when
> ata_host_start() itself failed.
> 
> Note that ata_pci_sff_activate_host() is deliberately left as is: none of
> its callers releases the host resources in its probe() error path, they
> all rely on ->host_stop() being called by devres, including
> ata_pci_init_one(), which releases the devres group of the host itself.
> 
> Fixes: 1896b15eddb4 ("ahci_platform: perform platform exit in host_stop() hook")
> Cc: stable@vger.kernel.org
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

  reply	other threads:[~2026-09-15  8:52 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-15  8:41 [PATCH v5 0/6] ata: Do not release the host resources twice on probe() failure Niklas Cassel
2026-09-15  8:41 ` [PATCH v5 1/6] ata: ahci_st: Do not ignore errors when getting the reset controls Niklas Cassel
2026-09-15  8:48   ` Damien Le Moal
2026-09-15  8:41 ` [PATCH v5 2/6] ata: ahci_st: Assert the power down reset in the probe() error path Niklas Cassel
2026-09-15  8:41 ` [PATCH v5 3/6] ata: sata_fsl: Fix use-after-free of host_priv on probe() failure Niklas Cassel
2026-09-15  8:50   ` Damien Le Moal
2026-09-15  9:49     ` Niklas Cassel
2026-09-15  8:41 ` [PATCH v5 4/6] ata: libahci: Free the IRQs when activating a multi-IRQ host fails Niklas Cassel
2026-09-15  8:41 ` [PATCH v5 5/6] ata: libata: Do not leave ata_host_stop() registered when activation fails Niklas Cassel
2026-09-15  8:52   ` Damien Le Moal [this message]
2026-09-15  8:41 ` [PATCH v5 6/6] ata: libata-core: Fix the ata_host_register() kdoc Niklas Cassel
2026-09-15  9:58 ` [PATCH v5 0/6] ata: Do not release the host resources twice on probe() failure Niklas Cassel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a478aaf0-5303-4cfe-a85a-3773f5cc9558@kernel.org \
    --to=dlemoal@kernel.org \
    --cc=cassel@kernel.org \
    --cc=computersforpeace@gmail.com \
    --cc=hansg@kernel.org \
    --cc=jgarzik@redhat.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.