Linux ATA/IDE development
 help / color / mirror / Atom feed
* [PATCH] ata: libata-core: Allow capacity transition to zero for locked drives
@ 2026-06-22 18:28 TJ Adams
  2026-06-22 18:40 ` sashiko-bot
  2026-06-24 14:20 ` Niklas Cassel
  0 siblings, 2 replies; 3+ messages in thread
From: TJ Adams @ 2026-06-22 18:28 UTC (permalink / raw)
  To: Damien Le Moal, Niklas Cassel, Hannes Reinecke,
	Martin K. Petersen
  Cc: linux-ide, linux-kernel, Igor Pylypiv, Salomon Dushimirimana,
	TJ Adams

Commit 91842ed844a0 ("ata: libata-core: Set capacity to zero for a
security locked drive") introduced setting the device capacity (n_sectors)
to zero in ata_dev_configure() if the drive is security locked.

However, during runtime revalidation, ata_dev_revalidate() compares the
new capacity (now 0) with the old capacity (>0) and detects a mismatch.
Since it does not consider the locked status, it returns -ENODEV.

This revalidation failure during Error Handling (EH) causes libata to
eventually disable the device, offlining the SCSI device and resulting
in subsequent accesses returning -ENXIO (No such device or address).

Fix this by allowing the capacity transition to
zero in ata_dev_revalidate() if the drive is reported as security locked by
ata_id_is_locked().

Fixes: 91842ed844a0 ("ata: libata-core: Set capacity to zero for a security locked drive")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Terrence Adams <tadamsjr@google.com>
---
 drivers/ata/libata-core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 3b6243f0f91e..d090667979ca 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3974,7 +3974,8 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
 
 	/* verify n_sectors hasn't changed */
 	if (dev->class != ATA_DEV_ATA || !n_sectors ||
-	    dev->n_sectors == n_sectors)
+	    dev->n_sectors == n_sectors ||
+	    (dev->n_sectors == 0 && ata_id_is_locked(dev->id)))
 		return 0;
 
 	/* n_sectors has changed */
-- 
2.55.0.rc0.786.g65d90a0328-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ata: libata-core: Allow capacity transition to zero for locked drives
  2026-06-22 18:28 [PATCH] ata: libata-core: Allow capacity transition to zero for locked drives TJ Adams
@ 2026-06-22 18:40 ` sashiko-bot
  2026-06-24 14:20 ` Niklas Cassel
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-06-22 18:40 UTC (permalink / raw)
  To: TJ Adams; +Cc: dlemoal, cassel, linux-ide

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

Pre-existing issues:
- [High] HPA resizing is permanently disabled (quirked) when a drive is initialized or revalidated while security-locked.
--

commit 2df5e6f4cdff68089316d7932aeee91f9d6633ae
Author: TJ Adams <tadamsjr@google.com>
ata: libata-core: Allow capacity transition to zero for locked drives

This commit fixes a revalidation failure that occurs when a security-locked
drive transitions its capacity to zero during Error Handling. It allows this
transition in ata_dev_revalidate() if the drive is reported as locked.

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index e76d15411e2ae..4241a71d1b5d1 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -3959,7 +3959,8 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
>  
>  	/* verify n_sectors hasn't changed */
>  	if (dev->class != ATA_DEV_ATA || !n_sectors ||
> -	    dev->n_sectors == n_sectors)
> +	    dev->n_sectors == n_sectors ||
> +	    (dev->n_sectors == 0 && ata_id_is_locked(dev->id)))

[Severity: High]
This is a pre-existing issue, but since this patch deals with revalidating
locked drives, is there a risk that ata_hpa_resize() permanently disables
HPA resizing for these drives?

Earlier in ata_dev_revalidate(), ata_dev_configure() is called, which then
calls ata_hpa_resize(). If the drive is locked, the command to read the
native max address fails with -EACCES, causing a sticky quirk to be set:

drivers/ata/libata-core.c:ata_hpa_resize() {
        ...
        rc = ata_read_native_max_address(dev, &native_sectors);
        if (rc) {
                ...
                } else if (rc == -EACCES || rc == -EPERM) {
                        ata_dev_warn(dev, "HPA support seems broken ...\n");
                        dev->quirks |= ATA_QUIRK_BROKEN_HPA;
                        return 0;
                }
        ...
}

Because the ATA_QUIRK_BROKEN_HPA quirk is sticky on the device, subsequent
revalidations after the drive is unlocked will silently bypass HPA checks:

drivers/ata/libata-core.c:ata_hpa_resize() {
        if (... || (dev->quirks & ATA_QUIRK_BROKEN_HPA))
                return 0;
        ...
}

Does this functionally prevent the unlocked drive from ever exposing its
full native capacity without a reboot or device removal?

>  		return 0;
>  
>  	/* n_sectors has changed */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622182844.2795777-1-tadamsjr@google.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ata: libata-core: Allow capacity transition to zero for locked drives
  2026-06-22 18:28 [PATCH] ata: libata-core: Allow capacity transition to zero for locked drives TJ Adams
  2026-06-22 18:40 ` sashiko-bot
@ 2026-06-24 14:20 ` Niklas Cassel
  1 sibling, 0 replies; 3+ messages in thread
From: Niklas Cassel @ 2026-06-24 14:20 UTC (permalink / raw)
  To: TJ Adams
  Cc: Damien Le Moal, Hannes Reinecke, Martin K. Petersen, linux-ide,
	linux-kernel, Igor Pylypiv, Salomon Dushimirimana

On Mon, Jun 22, 2026 at 11:28:44AM -0700, TJ Adams wrote:
> Commit 91842ed844a0 ("ata: libata-core: Set capacity to zero for a
> security locked drive") introduced setting the device capacity (n_sectors)
> to zero in ata_dev_configure() if the drive is security locked.
> 
> However, during runtime revalidation, ata_dev_revalidate() compares the
> new capacity (now 0) with the old capacity (>0) and detects a mismatch.
> Since it does not consider the locked status, it returns -ENODEV.

When booting with a security locked drive (that is not the boot device):

ata_eh_revalidate_and_attach()
 -> ata_dev_read_id()
 -> ata_dev_configure() <- sets dev->n_sectors to 0
 sets ATA_EHI_SETMODE

ata_dev_set_mode()
 -> ata_dev_revalidate()

ata_dev_revalidate()
 sets local variable n_sectors to dev->n_sectors

 -> ata_dev_reread_id()
     -> ata_dev_read_id()
 -> ata_dev_configure() will one again set dev->n_sectors to 0

  /* verify n_sectors hasn't changed */
  if (dev->class != ATA_DEV_ATA || !n_sectors ||
      dev->n_sectors == n_sectors)
          return 0;


Please explain how you reproduce this.
As far as I can see the local n_sectors variable will be 0,
and the function call to ata_dev_configure() will set dev->n_sectors
to 0, so this should, AFAICT, never get a "n_sectors mismatch" print.


I even tried to reproduce with doing a suspend + resume (using QEMU),
such that ata_dev_revalidate() will be called again:

$ sudo dmesg | grep -E "locked|suspend|disable device|ata_dev_revalidate"
[    0.988248] ata2.00: Security locked, setting capacity to zero
[    0.993442] ata1.00: debug print: ata_dev_revalidate
[    0.994729] ata2.00: debug print: ata_dev_revalidate
[    0.995918] ata2.00: Security locked, setting capacity to zero
[   40.239682] PM: suspend entry (deep)
[   40.300128] PM: suspend devices took 0.020 seconds
[   45.738412] PM: suspend exit
[   45.985092] ata2.00: debug print: ata_dev_revalidate
[   45.986698] ata2.00: Security locked, setting capacity to zero
[   45.988530] ata1.00: debug print: ata_dev_revalidate
[   45.993331] ata1.00: debug print: ata_dev_revalidate
[   45.994525] ata2.00: debug print: ata_dev_revalidate
[   45.995194] ata2.00: Security locked, setting capacity to zero


Since you seem to state that the old capacity (local variable n_sectors)
is > 0, it seems like the device wasn't locked during the initial boot /
ata_dev_configure() call.


Are you perhaps unlocking the device in firmware, so it is unlocked at
boot, and then when you suspend, you lose power to the drive, so when
you resume, the drive is locked?


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-24 14:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-22 18:28 [PATCH] ata: libata-core: Allow capacity transition to zero for locked drives TJ Adams
2026-06-22 18:40 ` sashiko-bot
2026-06-24 14:20 ` Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox