Re: [PATCH] usb: storage: sddr09: fix OOB access in sddr09_read_map

[Alan Stern]

On Fri, Aug 21, 2026 at 03:01:20PM +0800, Haofeng Li wrote:
> sddr09_read_map() builds the LBA <-> PBA translation tables while
> servicing READ_CAPACITY.  The logical block address assigned to each
> physical block is decoded from device-controlled redundancy data:
> 
> 	lba = short_pack(ptr[7], ptr[6]);	/* 16-bit device value */
> 	lba = (lba & 0x07FF) >> 1;		/* 0..1023 */
> 
> 	if (lba >= 1000)
> 		goto possibly_erase;
> 
> 	lba += 1000*(i/0x400);
> 
> 	if (info->lba_to_pba[lba] != UNDEF)	/* heap OOB read */
> 		...
> 	info->pba_to_lba[i] = lba;
> 	info->lba_to_pba[lba] = i;		/* heap OOB write */
> 
> The tables are allocated with numblocks entries each, where numblocks
> is derived from the device-reported NAND chip geometry.  For the
> smallest chip in nand_flash_ids[] (1 MB: chipshift=20, pageshift=8,
> blockshift=4):
> 
> 	numblocks = (1 << 20) >> (8 + 4) = 256
> 
> so info->lba_to_pba[] only has indexes 0..255, while a hostile device
> may report any LBA up to 999 - the "lba >= 1000" check is the only
> limit on the value, and there is no check that lba < numblocks.  On a
> 1 MB card indexes 256..999 (744 of them) index the table out of
> bounds, up to ~3 KB (744 * 4 bytes) past the end of the allocation.
> 
> Attack chain (malicious USB storage device):
> 
>   1. An attacker presents a USB Mass Storage device spoofing a unit
>      matched in sddr09_usb_ids, e.g. 0x04e6:0x0003 (SanDisk
>      ImageMate SDDR09) or 0x0781:0x0200, on the victim USB bus; the
>      device is enumerated as a normal storage device.
>   2. ums-sddr09 binds (sddr09_probe -> us->transport =
>      sddr09_transport) and the SCSI layer issues READ_CAPACITY, which
>      is handled via sddr09_get_cardinfo() (chip geometry from the
>      device ID, choosing numblocks) and sddr09_read_map().
>   3. The device reports a 1 MB chip (numblocks = 256) and fills the
>      per-block redundancy data with LBA values in the 256..999 range,
>      driving info->lba_to_pba[lba] and info->pba_to_lba[i] accesses
>      out of bounds: a heap OOB read used in the map-building
>      conditionals plus a heap OOB write of the loop index i (the
>      physical block number, 0..255) at an offset the attacker controls
>      via the reported LBA, corrupting adjacent heap memory.
> 
> The device-controlled inputs, the missing bound check and the OOB
> indexing are confirmed by end-to-end reproduction with a FunctionFS
> based malicious device emulator; the driver logged out-of-bounds
> indexes such as:
> 
> 	sddr09: LBA 256 seen for PBA -858993460 and 201
> 	sddr09: LBA 258 seen for PBA 4513 and 203
> 
> Add the missing bounds check: since lba is unsigned it can only be
> too large, so bail out with "lba >= numblocks" and mark the physical
> block UNUSABLE instead of indexing the tables.  This mirrors the
> max_lba bounds checks already applied to the SCSI-address-derived LBA
> in sddr09_read_data()/sddr09_write_data() and in the related sddr55
> and alauda drivers.
> 
> Signed-off-by: Haofeng Li <lihaofeng@xxxxxxxxxx>

This patch has all the signs of being written by an AI.  If it was, you 
need to include an Assisted-by: tag.

> ---
>  drivers/usb/storage/sddr09.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
> index 3d45e1b54c66..2d1ad10bd4cc 100644
> --- a/drivers/usb/storage/sddr09.c
> +++ b/drivers/usb/storage/sddr09.c
> @@ -1339,6 +1339,26 @@ sddr09_read_map(struct us_data *us) {
>  
>  		lba += 1000*(i/0x400);
>  
> +		/*
> +		 * The LBA is taken from device-controlled redundancy data
> +		 * and is only checked against the 1000-per-zone limit
> +		 * above.  Nothing prevents it from exceeding the size of
> +		 * the translation table, which for a 1 MB card has only
> +		 * numblocks = 256 entries while a device may report an LBA
> +		 * up to 999.  Bounds-check it before indexing
> +		 * info->lba_to_pba[]/info->pba_to_lba[], otherwise a
> +		 * hostile or corrupted card makes the driver read and
> +		 * write past the end of the table.
> +		 */

AI-generated comments are notoriously verbose.  Try to shrink this down 
to something a lot shorter.

> +		if (lba >= numblocks) {
> +			printk(KERN_WARNING
> +			       "sddr09: Bad LBA %d for block %d exceeds "
> +			       "the translation table size %d\n",
> +			       lba, i, numblocks);

Nowadays we have dev_warn() for this sort of thing.  And since the error 
is likely to crop up multiple times in a row, you might even want to use 
dev_warn_ratelimited().

Alan Stern

> +			info->pba_to_lba[i] = UNUSABLE;
> +			continue;
> +		}
> +
>  		if (info->lba_to_pba[lba] != UNDEF) {
>  			printk(KERN_WARNING
>  			       "sddr09: LBA %d seen for PBA %d and %d\n",
> -- 
> 2.25.1
>