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

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

[Felix Busch]

Signed-off-by: Felix Busch <felixbusch470@gmail.com>
---
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 

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;
 	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;
+	/* A special case upper bound check. */
+	if (cd_nr_sectors % blocksize == 0) {
+		unsigned int logical_blocks = cd_nr_sectors / blocksize;
+
+		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@gmail.com>

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@gmail.com>
> ---
> 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@gmail.com>
> 

Regards,
Phil

[PATCH v2 1/2] [PATCH] cdrom: extra upper bound check for logical block address.

[Felix Busch]

Hi Phillip,

Thank you for your feedback. I’ve updated everything based on your suggestions and hope
I’ve covered them all.

It has been built and run and tested in the following:

1) This has been tested on a real CD-ROM with 700MB capacity, containing 2097151
sectors in my case. Since 2097151 is not divisible by a blocksize of 2048, the if condition is not satisfied.

2) When using a smaller emulated CD-ROM with QEMU with the same blocksize,
the if condition is triggered when having a too big logical block addresses. For example, this occurs when minutes,
seconds, and frames are set to UINT8_MAX via an ioctl call.

According to this SCSI-2 spec, chapter 14 on this web page (date 23.2.2026):
https://www.staff.uni-mainz.de/tacke/scsi/SCSI2-introduction.html

They are stating:
"
Logical addressing 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 2 048 bytes are transferred from each CD-ROM sector (specified by the CD-ROM density code value),
and the logical block length is 512 bytes, then each CD-ROM sector shall map to exactly four logical blocks.
This International Standard does not define the mapping of logical block lengths which do not evenly divide
or are not exact multiples of the selected number of bytes per CD-ROM sector.
"

So, I do refer to this SCSI-2 spec.
Using this information it's possible to use logical_blocks = cd_nr_sectors / blocksize to get the number of blocks that are be able to read due to this mapping.
For now, I decided using this special checking only, due to the given statement in the SCSI-2 spec.

If lba > blocksize * (logical_blocks - 1), then it should not be possible to continue reading a full block, because then, as I understand,
it would "overflow" the region it can read from the disk.

Thanks for your time!

Felix


To: Phillip Potter <phil@philpotter.co.uk>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Felix Busch <felixbusch470@gmail.com>

---
Changes in v2:
- Reviewer feedback.
- Link to v1: https://lore.kernel.org/r/20260222-cdrom-additional-lba-check-v1-1-5f5e9f0c0fa4@gmail.com

--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
  "series": {
    "revision": 2,
    "change-id": "20260222-cdrom-additional-lba-check-2c88d18599d0",
    "prefixes": [],
    "history": {
      "v1": [
        "20260222-cdrom-additional-lba-check-v1-1-5f5e9f0c0fa4@gmail.com"
      ]
    }
  }
}
-- 
2.53.0

Re: [PATCH v2 1/2] [PATCH] cdrom: extra upper bound check for logical block address.

[Phillip Potter]

On Tue, Feb 24, 2026 at 05:16:12PM +0100, Felix Busch wrote:
> Hi Phillip,
> 
> Thank you for your feedback. I’ve updated everything based on your suggestions and hope
> I’ve covered them all.
> 
> It has been built and run and tested in the following:
> 
> 1) This has been tested on a real CD-ROM with 700MB capacity, containing 2097151
> sectors in my case. Since 2097151 is not divisible by a blocksize of 2048, the if condition is not satisfied.
> 
> 2) When using a smaller emulated CD-ROM with QEMU with the same blocksize,
> the if condition is triggered when having a too big logical block addresses. For example, this occurs when minutes,
> seconds, and frames are set to UINT8_MAX via an ioctl call.
> 
> According to this SCSI-2 spec, chapter 14 on this web page (date 23.2.2026):
> https://www.staff.uni-mainz.de/tacke/scsi/SCSI2-introduction.html
> 
> They are stating:
> "
> Logical addressing 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 2 048 bytes are transferred from each CD-ROM sector (specified by the CD-ROM density code value),
> and the logical block length is 512 bytes, then each CD-ROM sector shall map to exactly four logical blocks.
> This International Standard does not define the mapping of logical block lengths which do not evenly divide
> or are not exact multiples of the selected number of bytes per CD-ROM sector.
> "
> 
> So, I do refer to this SCSI-2 spec.
> Using this information it's possible to use logical_blocks = cd_nr_sectors / blocksize to get the number of blocks that are be able to read due to this mapping.
> For now, I decided using this special checking only, due to the given statement in the SCSI-2 spec.
> 
> If lba > blocksize * (logical_blocks - 1), then it should not be possible to continue reading a full block, because then, as I understand,
> it would "overflow" the region it can read from the disk.
>

I've seen your V3 - please make sure the relevant mailing list is copied
in when posting patches, as development is ideally done in the open. As
for that and for this version, I'm sorry but I'm still not convinced of
the approach unless I'm missing something.

You are taking units of two different things and dividing one by the
other. Dividing the number of sectors (2,097,151) by the detected
blocksize (2048 bytes) is not meaningful. The statement in the SCSI-2
spec simply refers to the difference between CD-ROM sector size in bytes
and the logical block size in bytes.

Here, you are dividing one thing (number of sectors) by another (block
size). Do you see what I mean?

Finally, please make sure cover letters of a patch series are labels
PATCH 0.

Regards,
Phil