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

On Sat, Aug 22, 2026 at 02:01:16AM +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 <[email protected]>
> Suggested-by: Alan Stern <[email protected]>
> Assisted-by: opencode:deepseek-v4-flash-free
> ---

Acked-by: Alan Stern <[email protected]>

> Changes in v2:
> - Shorten the overly verbose comment.
> - Replace printk(KERN_WARNING ...) with dev_warn_ratelimited().
> - Add an Assisted-by: tag.
> 
>  drivers/usb/storage/sddr09.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
> index 3d45e1b54c66..313313508bde 100644
> --- a/drivers/usb/storage/sddr09.c
> +++ b/drivers/usb/storage/sddr09.c
> @@ -1339,6 +1339,15 @@ sddr09_read_map(struct us_data *us) {
>  
>  		lba += 1000*(i/0x400);
>  
> +		/* The LBA comes from the card and may exceed the table size */
> +		if (lba >= numblocks) {
> +			dev_warn_ratelimited(&us->pusb_dev->dev,
> +					"sddr09: Bad LBA %d for block %d exceeds the translation table size %d\n",
> +					lba, i, numblocks);
> +			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