[PATCH] ata: libata-scsi: avoid READ CAPACITY 16 underflow

[PATCH] ata: libata-scsi: avoid READ CAPACITY 16 underflow

[shao.mingyin]

ata_scsiop_read_cap() starts with

	u64 last_lba = dev->n_sectors - 1;

ata_dev_configure() sets n_sectors to zero for security locked drives, to
prevent any I/O.  The READ CAPACITY 10 branch clamps the resulting
underflow to 0xffffffff (still wrong, but bounded), while READ CAPACITY 16
has no clamp and reports 0xffffffffffffffff, i.e. a capacity of about
8 ZiB, to the SCSI layer.

Use a zero last LBA when the device has no sectors.

Fixes: 61d79a8eb362 ("libata: Report disk alignment and physical block size")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a61fb0331279..eaf86dc2cefc 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2984,7 +2984,7 @@ static unsigned int ata_scsiop_read_cap(struct ata_device *dev,
 					struct scsi_cmnd *cmd, u8 *rbuf)
 {
 	u8 *scsicmd = cmd->cmnd;
-	u64 last_lba = dev->n_sectors - 1; /* LBA of the last block */
+	u64 last_lba = dev->n_sectors ? dev->n_sectors - 1 : 0; /* LBA of the last block */
 	u32 sector_size; /* physical sector size in bytes */
 	u8 log2_per_phys;
 	u16 lowest_aligned;

Re: [PATCH] ata: libata-scsi: avoid READ CAPACITY 16 underflow

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ata: libata-scsi: ata_scsiop_read_cap() reports 1 block when n_sectors is 0
--

commit 2baef3c64e10a35b658037f12f8a9575c158e978
Author: shao.mingyin@zte.com.cn <shao.mingyin@zte.com.cn>

ata: libata-scsi: avoid READ CAPACITY 16 underflow

This commit updates ata_scsiop_read_cap() to set last_lba to 0 instead of
underflowing dev->n_sectors - 1 when dev->n_sectors is zero, such as on
security-locked drives.

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index a61fb03312793..eaf86dc2cefc0 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2984,7 +2984,7 @@ static unsigned int ata_scsiop_read_cap(struct ata_device *dev,
>  					struct scsi_cmnd *cmd, u8 *rbuf)
>  {
>  	u8 *scsicmd = cmd->cmnd;
> -	u64 last_lba = dev->n_sectors - 1; /* LBA of the last block */
> +	u64 last_lba = dev->n_sectors ? dev->n_sectors - 1 : 0; /* LBA of the last block */

[Severity: High]
Does setting last_lba to 0 when dev->n_sectors is 0 cause the SCSI disk
driver to report a capacity of 1 block instead of 0 blocks?

In the SCSI READ CAPACITY (10) and READ CAPACITY (16) parameter data, the
returned logical block address is the 0-based LBA of the last logical block
on the medium (capacity - 1), so returning 0 indicates that LBA 0 is a valid
block on a 1-block disk.

When sd_revalidate_disk() -> sd_read_capacity() calls read_capacity_16() or
read_capacity_10() in drivers/scsi/sd.c, both functions add 1 to the
returned LBA:

drivers/scsi/sd.c:read_capacity_16() {
	...
	lba = get_unaligned_be64(&buffer[0]);
	...
	sdkp->capacity = lba + 1;
}

Previously, when dev->n_sectors was 0, last_lba underflowed to
0xffffffffffffffffULL, and lba + 1 in read_capacity_16() wrapped around in
64-bit unsigned arithmetic to set sdkp->capacity = 0.

With last_lba set to 0, sdkp->capacity becomes 1, and sd_revalidate_disk()
sets a non-zero gendisk capacity via set_capacity_and_notify(). In
drivers/ata/libata-core.c, ata_dev_configure() sets dev->n_sectors = 0 on
security-locked drives specifically to prevent partition scanning:

drivers/ata/libata-core.c:ata_dev_configure() {
	...
		if (ata_id_is_locked(id)) {
			/*
			 * If Security locked, set capacity to zero to prevent
			 * any I/O, e.g. partition scanning, as any I/O to a
			 * locked drive will result in user visible errors.
			 */
			ata_dev_info(dev,
				"Security locked, setting capacity to zero\n");
			dev->n_sectors = 0;
		}
}

Because bdev_disk_changed() in block/partitions/core.c checks
if (get_capacity(disk)) before calling blk_add_partitions(disk), won't a
reported capacity of 1 block cause the block layer to issue partition-scan
reads to LBA 0 on security-locked drives, resulting in I/O errors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103185346937YqEZr3t5lq7FdnTzM3-TY@zte.com.cn?part=1

Re: [PATCH] ata: libata-scsi: avoid READ CAPACITY 16 underflow

[shao.mingyin]

You are right, and thanks for catching it.  Please drop this patch.

The underflow to 0xffff_ffff_ffff_ffff is what keeps the capacity at
zero: sd_read_capacity_16() computes sdkp->capacity = lba + 1, so the
value wraps before it reaches the block layer, and the "8 ZiB" in my
changelog was wrong for the same reason.  Reporting 0 turns a locked
drive into a one block disk and brings the partition scan back.

Sorry about the noise.