Re: [PATCH] ata: libata-core: Allow capacity transition to zero for locked drives

[Niklas Cassel <cassel@kernel.org>]

Hello TJ,

On Wed, Jul 01, 2026 at 04:16:49PM -0700, TJ Adams wrote:
> On Wed, Jun 24, 2026 at 7:21 AM Niklas Cassel <cassel@kernel.org> wrote:
> >
> > 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.
> 
> 
> Hey Niklas,
> 
> Sorry for the delay in response. I had some difficulty recreating the
> issue outside of our specific test suite.
> 
> > 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.
> 
> Here are the steps to recreate it:
> 
> ```bash
> TARGET_DEV="/dev/sdy"
> TARGET_NAME="sdy"
> 
> # Lock Device
> hdparm --user-master u --security-set-pass TempPassword "$TARGET_DEV"
> 
> # Disable SSP
> sg_raw "$TARGET_DEV" 85 06 00 00 90 00 06 00 00 00 00 00 00 00 ef 00
> 
> # In another terminal set active I/O
> dd if="$TARGET_DEV" of=/dev/null bs=1M status=progress
> 
> # Do an active reset (then wait for scsi eh)
> echo 1 > /sys/class/sas_phy/phy-X:Y/hard_reset
> 
> # You can end the dd command in the other terminal and then run this:
> # You should see it fail immediately
> dd if="$TARGET_DEV" of=/dev/null bs=512 count=1
> ```
> 
> Some things to note:
> - Disabling SSP. I had some attempts to recreate the bug but didn't
>   realize SSP was enabled. I would lock the drive but on reset the
>   drive would actually come back up unlocked, masking the bug.
> - The active I/O. This is so there are I/Os in flight whenever the
>   drive gets reset. They should timeout and then trigger the error
>   handling path which will cause the ata device revalidation. Without
>   it the revalidation might not occur and you might not see the bug.
>   I also experienced this.
> 
> > 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.
> 
> Yeah that's correct. To recreate the issue you should boot unlocked but
> lock at runtime.
> 
> Also here is a dmesg snippet showing the capacity mismatch:
> 
> ```dmesg
>   [76834.784699] ata36.00: status: { DRDY }
>   [76834.784708] ata36: hard resetting link
>   [76834.940530] ata36.00: supports DRM functions and may not be fully
> accessible
>   [76834.940537] ata36.00: Security locked, setting capacity to zero
>   [76834.943566] ata36.00: n_sectors mismatch 15628053168 != 0
> ```
> 
> Let me know if I didn't explain something well or if something is
> missing. Thank you!

Ok, thank you for the information.


Perhaps mention more clearly in the commit message that this happens
when doing a reset of the PHY for a controller that has I/Os in flight.


I think a simpler patch would be:

@@ -3959,7 +3959,7 @@ 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 || ata_id_is_locked(dev->id))
                return 0;
 
        /* n_sectors has changed */


I don't see why we need the additional dev->n_sectors == 0.
If the drive is locked, no need to contiunue, just return.

Perhaps you could test that change instead?


Also, I think we should have a patch 1/2 (or 2/2) that does:


@@ -1338,7 +1338,7 @@ static int ata_hpa_resize(struct ata_device *dev)
        /* do we need to do it? */
        if ((dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) ||
            !ata_id_has_lba(dev->id) || !ata_id_hpa_enabled(dev->id) ||
-           (dev->quirks & ATA_QUIRK_BROKEN_HPA))
+           (dev->quirks & ATA_QUIRK_BROKEN_HPA) || ata_id_is_locked(dev->id))
                return 0;
 
        /* read native max address */


To fix the problem Sashiko complained about.


Kind regards,
Niklas