Re: [PATCH] Added special upper bound check for the logical block address in mmc_ioctl_cdrom_read_data()

[Phillip Potter]

Hi Felix,

Thank you for your patch firstly. First issue however is it doesn't conform to
the required uniform-diff format such that it can be applied by tools such as
'git am'. The patch e-mail should use a subject of the form (for example):

[PATCH] cdrom: short imperative description

With a longer description such as the one you've supplied, imperatively,
followed by your Signed-off-by tag.

Please review this for additional guidance:
https://docs.kernel.org/process/submitting-patches.html

Onto the patch itself...

On Sun, Feb 22, 2026 at 05:41:46PM +0100, Felix Busch wrote:
> Signed-off-by: Felix Busch <felixbusch470@xxxxxxxxx>
> ---
> This patch contains an extra check on the comment 
> for an upper bound check for the logical block address in the 
> function mmc_ioctl_cdrom_read_data(). 
> 
> This web page: 
> http://www.o3one.org/hwdocs/cdrom_formats/scsi_programming.htm 

'This webpage' is evidently a reference to the SCSI-2 spec, but this
should be clearer in the description if it's being referenced. Its
veracity should also be considered (although this information seems
to appear verbatim on a number of sites).

> 
> states that:
> "Logical adressing of CD-ROM information may use any logical
> block length. When the specified logical block length is an
> exact divisor or integral multiple of the selected number 
> of bytes per CD-ROM sector, the device shall map (one to one)
> the bytes transferred from CD-ROM sectors to the bytes of logical 
> blocks. For instance, if 2048 bytes are transferred from each 
> CD-ROM sector,.., and the logical block length is 512 bytes, then
> each CD-ROM sector shall map to exactly four logical blocks."
> 
> If the number of sectors on the CD drive is a multiple of the block
> size, then, as I understand, it should be possible to perform a 
> simple check on the logical block address in this case.
> 
> If the logical block address (lba value) is greater than
>  (logical_blocks-1) * blocksize, then it should not be possible 
> to read the next block, because if the lba value is greater
> than this value, then it might try to read a full next block that 
> does not exist.
> 
> Please let me know what you think.
> Thank you very much.
> ---
>  drivers/cdrom/cdrom.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 31ba1f8c1f78..0149813ef903 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -2927,6 +2927,7 @@ static noinline int mmc_ioctl_cdrom_read_data(struct cdrom_device_info *cdi,
>  	struct scsi_sense_hdr sshdr;
>  	struct cdrom_msf msf;
>  	int blocksize = 0, format = 0, lba;
> +	unsigned int cd_nr_sectors;

This should be sector_t strictly speaking.

>  	int ret;
>  
>  	switch (cmd) {
> @@ -2945,9 +2946,19 @@ static noinline int mmc_ioctl_cdrom_read_data(struct cdrom_device_info *cdi,
>  		return -EFAULT;
>  	lba = msf_to_lba(msf.cdmsf_min0, msf.cdmsf_sec0, msf.cdmsf_frame0);
>  	/* FIXME: we need upper bound checking, too!! */
> +	/* Lower bound check for logical block address. */
>  	if (lba < 0)
>  		return -EINVAL;
>  
> +	cd_nr_sectors = cdi->disk->part0->bd_nr_sectors;

There is a utility function for reading number of sectors. That (to me at
least) seems preferable to accessing this field directly.

> +	/* A special case upper bound check. */
> +	if (cd_nr_sectors % blocksize == 0) {
> +		unsigned int logical_blocks = cd_nr_sectors / blocksize;

Again this should be sector_t. I'm not even sure this is correct though.
We are taking the number of sectors and dividing it by the block size
determined above based on read mode of RAW/1/2. Perhaps my eyes are just
tired, but how does that give us the number of logical blocks? It seems
this code confuses sector size with number of sectors unless I'm missing
something.

Has it been built/run/tested?

> +
> +		if (lba > blocksize * (logical_blocks - 1))
> +			return -EINVAL;
> +	}
> +
>  	cgc->buffer = kzalloc(blocksize, GFP_KERNEL);
>  	if (cgc->buffer == NULL)
>  		return -ENOMEM;
> 
> ---
> base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
> change-id: 20260222-cdrom-additional-lba-check-2c88d18599d0
> 
> Best regards,
> -- 
> Felix Busch <felixbusch470@xxxxxxxxx>
> 

Regards,
Phil