Re: [PATCH 1/1] CD-ROM: Additional LBA bound check

[James Bottomley]

On Wed, 2026-03-25 at 08:44 +0100, Felix Busch wrote:
[...]
> mmc_ioctl_cdrom_read_data(struct cdrom_device_info *cdi,
>  	if (copy_from_user(&msf, (struct cdrom_msf __user *)arg,
> sizeof(msf)))
>  		return -EFAULT;
>  	lba = msf_to_lba(msf.cdmsf_min0, msf.cdmsf_sec0,
> msf.cdmsf_frame0);
> -	/* FIXME: we need upper bound checking, too!! */
> -	if (lba < 0)
> +	nr_blocks = cdo->get_capacity(cdi);

Since you only give sr.c a get_capacity method, doesn't this crash for
every other CD backend?

[...]

> @@ -782,7 +792,7 @@ static int get_sectorsize(struct scsi_cd *cd)
>  			sector_size = 2048;
>  			fallthrough;
>  		case 2048:
> -			cd->capacity *= 4;
> +			//cd->capacity *= 4;
>  			fallthrough;

You mentioned this in the cover letter.  It's because of the
		set_capacity(cd->disk, cd->capacity); below.  The block layer always speaks512bytesectorsforcapacities.Sothiswillbeabreakingchangeforlargesectordevicesbecausethey'llappearfourtimessmallertotheblocklayer.

I suppose I'm also a bit hazy about the actual justification.  Today if
you send an over capacity request via the ioctl, you'll get an error
from the device.  If we apply the patch you'll get a -EINVAL instead. 
So you get an error in both cases (i.e. the macro behaviour doesn't
change, so it's hard to justify why we need the patch) but a different
one, which might confuse some tools ... have you checked?

The cover letter says "improve execution performance" but I can't see
how introducing an indirect call into the read path can do that.  I
can't actually see why you need an indirect call since the capacity
doesn't change except if the device is writeable and rewritten.

Regards,

James