[PATCH v2] cdrom: fix stack memory leaks in ioctl handlers

[PATCH v2] cdrom: fix stack memory leaks in ioctl handlers

[Sreeraj S Kurup]

Multiple ioctl handlers in drivers/cdrom/cdrom.c allocate structures on
the kernel stack that are subsequently copied back to user space via
copy_to_user(). Uninitialized structure padding bytes or unpopulated
fields in these stack-allocated structures leak sensitive kernel stack
memory to user space.

Explicitly zero-initialize local structures across the affected output
ioctl handlers using standard {0} initialization:

- cdrom_ioctl_multisession: struct cdrom_multisession info
- cdrom_ioctl_timed_media_change: struct cdrom_timed_media_change_info
- cdrom_ioctl_get_mcn: struct cdrom_mcn mcn
- cdrom_ioctl_get_subchnl: struct cdrom_subchnl q
- cdrom_ioctl_read_tochdr: struct cdrom_tochdr header
- cdrom_ioctl_read_tocentry: struct cdrom_tocentry entry
- mmc_ioctl_cdrom_subchannel: struct cdrom_subchnl q

This prevents kernel stack memory disclosure vulnerabilities when
copying data structures back to user space.

---
v2:
- Expanded coverage to all remaining ioctl handlers in cdrom.c that
  copy stack structures back to user space.

Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>
---
 drivers/cdrom/cdrom.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 4f1fd389260f..85be658bbfa3 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -2254,7 +2254,7 @@ EXPORT_SYMBOL_GPL(cdrom_multisession);
 static int cdrom_ioctl_multisession(struct cdrom_device_info *cdi,
 		void __user *argp)
 {
-	struct cdrom_multisession info;
+	struct cdrom_multisession info = {0};
 	int ret;
 
 	cd_dbg(CD_DO_IOCTL, "entering CDROMMULTISESSION\n");
@@ -2361,7 +2361,7 @@ static int cdrom_ioctl_timed_media_change(struct cdrom_device_info *cdi,
 {
 	int ret;
 	struct cdrom_timed_media_change_info __user *info;
-	struct cdrom_timed_media_change_info tmp_info;
+	struct cdrom_timed_media_change_info tmp_info = {0};
 
 	if (!CDROM_CAN(CDC_MEDIA_CHANGED))
 		return -ENOSYS;
@@ -2510,7 +2510,7 @@ static int cdrom_ioctl_get_capability(struct cdrom_device_info *cdi)
 static int cdrom_ioctl_get_mcn(struct cdrom_device_info *cdi,
 		void __user *argp)
 {
-	struct cdrom_mcn mcn;
+	struct cdrom_mcn mcn = {0};
 	int ret;
 
 	cd_dbg(CD_DO_IOCTL, "entering CDROM_GET_MCN\n");
@@ -2598,7 +2598,7 @@ static int cdrom_ioctl_changer_nslots(struct cdrom_device_info *cdi)
 static int cdrom_ioctl_get_subchnl(struct cdrom_device_info *cdi,
 		void __user *argp)
 {
-	struct cdrom_subchnl q;
+	struct cdrom_subchnl q = {0};
 	u8 requested, back;
 	int ret;
 
@@ -2629,7 +2629,7 @@ static int cdrom_ioctl_get_subchnl(struct cdrom_device_info *cdi,
 static int cdrom_ioctl_read_tochdr(struct cdrom_device_info *cdi,
 		void __user *argp)
 {
-	struct cdrom_tochdr header;
+	struct cdrom_tochdr header = {0};
 	int ret;
 
 	/* cd_dbg(CD_DO_IOCTL, "entering CDROMREADTOCHDR\n"); */
@@ -2669,7 +2669,7 @@ EXPORT_SYMBOL_GPL(cdrom_read_tocentry);
 static int cdrom_ioctl_read_tocentry(struct cdrom_device_info *cdi,
 		void __user *argp)
 {
-	struct cdrom_tocentry entry;
+	struct cdrom_tocentry entry = {0};
 	int ret;
 
 	if (copy_from_user(&entry, argp, sizeof(entry)))
@@ -3055,7 +3055,7 @@ static noinline int mmc_ioctl_cdrom_subchannel(struct cdrom_device_info *cdi,
 					       void __user *arg)
 {
 	int ret;
-	struct cdrom_subchnl q;
+	struct cdrom_subchnl q = {0};
 	u_char requested, back;
 	if (copy_from_user(&q, (struct cdrom_subchnl __user *)arg, sizeof(q)))
 		return -EFAULT;
-- 
2.54.0

Re: [PATCH v2] cdrom: fix stack memory leaks in ioctl handlers

[Phillip Potter]

On Tue, Aug 04, 2026 at 04:47:28PM +0000, Sreeraj S Kurup wrote:
> Multiple ioctl handlers in drivers/cdrom/cdrom.c allocate structures on
> the kernel stack that are subsequently copied back to user space via
> copy_to_user(). Uninitialized structure padding bytes or unpopulated
> fields in these stack-allocated structures leak sensitive kernel stack
> memory to user space.
> 
> Explicitly zero-initialize local structures across the affected output
> ioctl handlers using standard {0} initialization:
> 
> - cdrom_ioctl_multisession: struct cdrom_multisession info
> - cdrom_ioctl_timed_media_change: struct cdrom_timed_media_change_info
> - cdrom_ioctl_get_mcn: struct cdrom_mcn mcn
> - cdrom_ioctl_get_subchnl: struct cdrom_subchnl q
> - cdrom_ioctl_read_tochdr: struct cdrom_tochdr header
> - cdrom_ioctl_read_tocentry: struct cdrom_tocentry entry
> - mmc_ioctl_cdrom_subchannel: struct cdrom_subchnl q
> 
> This prevents kernel stack memory disclosure vulnerabilities when
> copying data structures back to user space.
> 
> ---
> v2:
> - Expanded coverage to all remaining ioctl handlers in cdrom.c that
>   copy stack structures back to user space.
> 
> Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>
> ---
>  drivers/cdrom/cdrom.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 4f1fd389260f..85be658bbfa3 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -2254,7 +2254,7 @@ EXPORT_SYMBOL_GPL(cdrom_multisession);
>  static int cdrom_ioctl_multisession(struct cdrom_device_info *cdi,
>  		void __user *argp)
>  {
> -	struct cdrom_multisession info;
> +	struct cdrom_multisession info = {0};
>  	int ret;
>  
>  	cd_dbg(CD_DO_IOCTL, "entering CDROMMULTISESSION\n");
> @@ -2361,7 +2361,7 @@ static int cdrom_ioctl_timed_media_change(struct cdrom_device_info *cdi,
>  {
>  	int ret;
>  	struct cdrom_timed_media_change_info __user *info;
> -	struct cdrom_timed_media_change_info tmp_info;
> +	struct cdrom_timed_media_change_info tmp_info = {0};
>  
>  	if (!CDROM_CAN(CDC_MEDIA_CHANGED))
>  		return -ENOSYS;
> @@ -2510,7 +2510,7 @@ static int cdrom_ioctl_get_capability(struct cdrom_device_info *cdi)
>  static int cdrom_ioctl_get_mcn(struct cdrom_device_info *cdi,
>  		void __user *argp)
>  {
> -	struct cdrom_mcn mcn;
> +	struct cdrom_mcn mcn = {0};
>  	int ret;
>  
>  	cd_dbg(CD_DO_IOCTL, "entering CDROM_GET_MCN\n");
> @@ -2598,7 +2598,7 @@ static int cdrom_ioctl_changer_nslots(struct cdrom_device_info *cdi)
>  static int cdrom_ioctl_get_subchnl(struct cdrom_device_info *cdi,
>  		void __user *argp)
>  {
> -	struct cdrom_subchnl q;
> +	struct cdrom_subchnl q = {0};
>  	u8 requested, back;
>  	int ret;
>  
> @@ -2629,7 +2629,7 @@ static int cdrom_ioctl_get_subchnl(struct cdrom_device_info *cdi,
>  static int cdrom_ioctl_read_tochdr(struct cdrom_device_info *cdi,
>  		void __user *argp)
>  {
> -	struct cdrom_tochdr header;
> +	struct cdrom_tochdr header = {0};
>  	int ret;
>  
>  	/* cd_dbg(CD_DO_IOCTL, "entering CDROMREADTOCHDR\n"); */
> @@ -2669,7 +2669,7 @@ EXPORT_SYMBOL_GPL(cdrom_read_tocentry);
>  static int cdrom_ioctl_read_tocentry(struct cdrom_device_info *cdi,
>  		void __user *argp)
>  {
> -	struct cdrom_tocentry entry;
> +	struct cdrom_tocentry entry = {0};
>  	int ret;
>  
>  	if (copy_from_user(&entry, argp, sizeof(entry)))
> @@ -3055,7 +3055,7 @@ static noinline int mmc_ioctl_cdrom_subchannel(struct cdrom_device_info *cdi,
>  					       void __user *arg)
>  {
>  	int ret;
> -	struct cdrom_subchnl q;
> +	struct cdrom_subchnl q = {0};
>  	u_char requested, back;
>  	if (copy_from_user(&q, (struct cdrom_subchnl __user *)arg, sizeof(q)))
>  		return -EFAULT;
> -- 
> 2.54.0
> 

Hi Sreeraj,

Thanks again for this and your previous patch. I've now had a look at this,
and I would have to politely dispute the 'fixing of memory leaks' here.

All of these structures (with the exception of struct cdrom_mcn mcn) are
effectively immediately fed into copy_from_user(...), which should be
overwriting the entire structure (including padding) with bytes from the
provided userspace pointer. If this fails, the functions bail
immediately.

As for 'struct cdrom_mcn mcn', this structure is fed to an ioctl
function pointer which ultimately ends up invoking sr_get_mcn(...) which
populates the first 13 bytes of the array and zero terminates it by
setting the final byte to 0. If this function fails,
cdrom_ioctl_get_mcn(...) likewise bails.

There are thus no vulnerabilities to be fixed here. There is no possible
case (unless I'm missing it) in which any of these lines in their
current state could lead to kernel stack memory being leaked back to
userspace. For that reason, I'm not going to take this patch as it isn't
fixing anything.

While I agree with defensive programming in general, these cases have
already been handled without explicit stack initialisation.

All the best.

Regards,
Phil