* [PATCH] cdrom: fix stack out-of-bounds read in CDROMVOLCTRL
@ 2026-07-13 8:20 raoxu
2026-07-17 7:11 ` Phillip Potter
2026-07-19 18:40 ` Phillip Potter
0 siblings, 2 replies; 4+ messages in thread
From: raoxu @ 2026-07-13 8:20 UTC (permalink / raw)
To: phil; +Cc: linux-kernel, raoxu, stable
From: Xu Rao <raoxu@uniontech.com>
mmc_ioctl_cdrom_volume() first reads the audio control mode page into a
32-byte stack buffer with cgc->buflen set to 24. If the device reports a
block descriptor, the function increases cgc->buflen to include that
descriptor and reads the page again.
For CDROMVOLCTRL, the function then builds a MODE SELECT parameter list
by moving cgc->buffer forward by offset - 8 bytes. This drops the block
descriptor from the outgoing payload and leaves a new 8-byte mode
parameter header in front of the audio control page. However, cgc->buflen
is left unchanged.
With a standard 8-byte block descriptor, cgc->buffer points at buffer + 8
but cgc->buflen remains 32. cdrom_mode_select() therefore asks the low
level packet path to write 32 bytes from that adjusted pointer, reading 8
bytes past the end of the 32-byte stack buffer.
This is not hit by CDROMVOLREAD, and CDROMVOLCTRL only triggers it on
drives that return a non-zero block descriptor length, which helps explain
why it has gone unnoticed. The overread is also sent to the device as
extra MODE SELECT payload, so it may not produce an obvious local failure.
Reduce cgc->buflen by the same amount as the buffer pointer adjustment so
the MODE SELECT transfer covers only the intended parameter list.
Fixes: 3147c531b6b5 ("cdrom: split mmc_ioctl to lower stack usage")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
drivers/cdrom/cdrom.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 62934cf4b10d..4f1fd389260f 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -3187,6 +3187,7 @@ static noinline int mmc_ioctl_cdrom_volume(struct cdrom_device_info *cdi,
/* set volume */
cgc->buffer = buffer + offset - 8;
+ cgc->buflen -= offset - 8;
memset(cgc->buffer, 0, 8);
return cdrom_mode_select(cdi, cgc);
}
--
2.50.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cdrom: fix stack out-of-bounds read in CDROMVOLCTRL
2026-07-13 8:20 [PATCH] cdrom: fix stack out-of-bounds read in CDROMVOLCTRL raoxu
@ 2026-07-17 7:11 ` Phillip Potter
2026-07-19 18:40 ` Phillip Potter
1 sibling, 0 replies; 4+ messages in thread
From: Phillip Potter @ 2026-07-17 7:11 UTC (permalink / raw)
To: raoxu; +Cc: linux-kernel, stable, phil
On Mon, Jul 13, 2026 at 04:20:13PM +0800, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
>
> mmc_ioctl_cdrom_volume() first reads the audio control mode page into a
> 32-byte stack buffer with cgc->buflen set to 24. If the device reports a
> block descriptor, the function increases cgc->buflen to include that
> descriptor and reads the page again.
>
> For CDROMVOLCTRL, the function then builds a MODE SELECT parameter list
> by moving cgc->buffer forward by offset - 8 bytes. This drops the block
> descriptor from the outgoing payload and leaves a new 8-byte mode
> parameter header in front of the audio control page. However, cgc->buflen
> is left unchanged.
>
> With a standard 8-byte block descriptor, cgc->buffer points at buffer + 8
> but cgc->buflen remains 32. cdrom_mode_select() therefore asks the low
> level packet path to write 32 bytes from that adjusted pointer, reading 8
> bytes past the end of the 32-byte stack buffer.
>
> This is not hit by CDROMVOLREAD, and CDROMVOLCTRL only triggers it on
> drives that return a non-zero block descriptor length, which helps explain
> why it has gone unnoticed. The overread is also sent to the device as
> extra MODE SELECT payload, so it may not produce an obvious local failure.
>
> Reduce cgc->buflen by the same amount as the buffer pointer adjustment so
> the MODE SELECT transfer covers only the intended parameter list.
>
> Fixes: 3147c531b6b5 ("cdrom: split mmc_ioctl to lower stack usage")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
> drivers/cdrom/cdrom.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 62934cf4b10d..4f1fd389260f 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -3187,6 +3187,7 @@ static noinline int mmc_ioctl_cdrom_volume(struct cdrom_device_info *cdi,
>
> /* set volume */
> cgc->buffer = buffer + offset - 8;
> + cgc->buflen -= offset - 8;
> memset(cgc->buffer, 0, 8);
> return cdrom_mode_select(cdi, cgc);
> }
> --
> 2.50.1
>
Hi Xu Rao,
Thanks for your patch. I will endeavour to look at it and verify it this
weekend and come back to you.
Regards,
Phil
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cdrom: fix stack out-of-bounds read in CDROMVOLCTRL
2026-07-13 8:20 [PATCH] cdrom: fix stack out-of-bounds read in CDROMVOLCTRL raoxu
2026-07-17 7:11 ` Phillip Potter
@ 2026-07-19 18:40 ` Phillip Potter
2026-07-20 1:31 ` raoxu
1 sibling, 1 reply; 4+ messages in thread
From: Phillip Potter @ 2026-07-19 18:40 UTC (permalink / raw)
To: raoxu; +Cc: phil, linux-kernel, stable
On Mon, Jul 13, 2026 at 04:20:13PM +0800, raoxu wrote:
> From: Xu Rao <raoxu@uniontech.com>
>
> mmc_ioctl_cdrom_volume() first reads the audio control mode page into a
> 32-byte stack buffer with cgc->buflen set to 24. If the device reports a
> block descriptor, the function increases cgc->buflen to include that
> descriptor and reads the page again.
>
> For CDROMVOLCTRL, the function then builds a MODE SELECT parameter list
> by moving cgc->buffer forward by offset - 8 bytes. This drops the block
> descriptor from the outgoing payload and leaves a new 8-byte mode
> parameter header in front of the audio control page. However, cgc->buflen
> is left unchanged.
>
> With a standard 8-byte block descriptor, cgc->buffer points at buffer + 8
> but cgc->buflen remains 32. cdrom_mode_select() therefore asks the low
> level packet path to write 32 bytes from that adjusted pointer, reading 8
> bytes past the end of the 32-byte stack buffer.
>
> This is not hit by CDROMVOLREAD, and CDROMVOLCTRL only triggers it on
> drives that return a non-zero block descriptor length, which helps explain
> why it has gone unnoticed. The overread is also sent to the device as
> extra MODE SELECT payload, so it may not produce an obvious local failure.
>
> Reduce cgc->buflen by the same amount as the buffer pointer adjustment so
> the MODE SELECT transfer covers only the intended parameter list.
>
> Fixes: 3147c531b6b5 ("cdrom: split mmc_ioctl to lower stack usage")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Rao <raoxu@uniontech.com>
> ---
> drivers/cdrom/cdrom.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 62934cf4b10d..4f1fd389260f 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -3187,6 +3187,7 @@ static noinline int mmc_ioctl_cdrom_volume(struct cdrom_device_info *cdi,
>
> /* set volume */
> cgc->buffer = buffer + offset - 8;
> + cgc->buflen -= offset - 8;
> memset(cgc->buffer, 0, 8);
> return cdrom_mode_select(cdi, cgc);
> }
> --
> 2.50.1
>
So I've looked at the patch and surrounding code in more detail now,
certainly seems correct to me - nice spot.
I can confirm I've build and boot tested it too, and for what it's
worth, I can still set the volume correctly with a simple C binary I put
together.
One thing however - I don't think your Fixes tag is correct. I went back
and looked at this commit (3147c531b6b5), and the problem is still
technically present even prior to this refactor, all the way back to the
initial Linux git commit (1da177e4c3f4).
Are you happy for me to adjust this therefore? Assuming you are, I'll
pass this up for inclusion with the adjusted Fixes tag. Let me know.
Regards,
Phil
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cdrom: fix stack out-of-bounds read in CDROMVOLCTRL
2026-07-19 18:40 ` Phillip Potter
@ 2026-07-20 1:31 ` raoxu
0 siblings, 0 replies; 4+ messages in thread
From: raoxu @ 2026-07-20 1:31 UTC (permalink / raw)
To: phil; +Cc: linux-kernel, raoxu, stable
Hi Phil,
> One thing however - I don't think your Fixes tag is correct. I went back
> and looked at this commit (3147c531b6b5), and the problem is still
> technically present even prior to this refactor, all the way back to the
> initial Linux git commit (1da177e4c3f4).
>
> Are you happy for me to adjust this therefore? Assuming you are, I'll
> pass this up for inclusion with the adjusted Fixes tag. Let me know.
Yes, please go ahead and adjust the Fixes tag to point to the initial
Linux Git commit.
Thank you for the detailed review, build and boot testing, and for
passing the patch on for inclusion.
Regards,
Xu Rao
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-20 1:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-13 8:20 [PATCH] cdrom: fix stack out-of-bounds read in CDROMVOLCTRL raoxu
2026-07-17 7:11 ` Phillip Potter
2026-07-19 18:40 ` Phillip Potter
2026-07-20 1:31 ` raoxu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome