Re: cdrom: cdrom_mrw_exit() NULL ptr deref

[Phillip Potter <phil@philpotter.co.uk>]

On Mon, Jul 14, 2025 at 08:22:11AM -0600, Jens Axboe wrote:
> On 7/11/25 2:46 PM, Phillip Potter wrote:
> >> <1>[335443.339244] BUG: kernel NULL pointer dereference, address: 0000000000000010
> >> <1>[335443.339262] #PF: supervisor read access in kernel mode
> >> <1>[335443.339268] #PF: error_code(0x0000) - not-present page
> >> <6>[335443.339273] PGD 0 P4D 0
> >> <4>[335443.339279] Oops: 0000 [#1] PREEMPT SMP NOPTI
> >> <4>[335443.339287] CPU: 1 PID: 1988 Comm: cros-disks Not tainted 6.6.76-07501-gd42535a678fb #1 (HASH:7d84 1)
> >> <4>[335443.339301] RIP: 0010:blk_queue_enter+0x5a/0x250
> >> <4>[335443.339312] Code: 03 00 00 4c 8d 6d a8 eb 1c 4c 89 e7 4c 89 ee e8 8c 62 be ff 49 f7 86 88 00 00 00 02 00 00 00 0f 85 ce 01 00 00 e8 86 10 bd ff <49> 8b 07 a8 03 0f 85 85 01 00 00 65 48 ff 00 41 83 be 90 00 00 00
> >> <4>[335443.339318] RSP: 0018:ffff9be08ab03b00 EFLAGS: 00010202
> >> <4>[335443.339324] RAX: ffff8903aa366300 RBX: 0000000000000000 RCX: ffff9be08ab03cd0
> >> <4>[335443.339330] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> >> <4>[335443.339333] RBP: ffff9be08ab03b58 R08: 0000000000000002 R09: 0000000000001b58
> >> <4>[335443.339338] R10: ffffffff00000000 R11: ffffffffc0ccd030 R12: 0000000000000328
> >> <4>[335443.339344] R13: ffff9be08ab03b00 R14: 0000000000000000 R15: 0000000000000010
> >> <4>[335443.339348] FS: 00007d52be81e900(0000) GS:ffff8904b6040000(0000) knlGS:0000000000000000
> >> <4>[335443.339357] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> <4>[335443.339362] CR2: 0000000000000010 CR3: 0000000140ac6000 CR4: 0000000000350ee0
> >> <4>[335443.339367] Call Trace:
> >> <4>[335443.339372] <TASK>
> >> <4>[335443.339379] ? __die_body+0xae/0xb0
> >> <4>[335443.339389] ? page_fault_oops+0x381/0x3e0
> >> <4>[335443.339398] ? exc_page_fault+0x4f/0xa0
> >> <4>[335443.339404] ? asm_exc_page_fault+0x22/0x30
> >> <4>[335443.339416] ? sr_check_events+0x290/0x290 [sr_mod (HASH:ab3e 2)]
> >> <4>[335443.339432] ? blk_queue_enter+0x5a/0x250
> >> <4>[335443.339439] blk_mq_alloc_request+0x16a/0x220
> >> <4>[335443.339450] scsi_execute_cmd+0x65/0x240
> >> <4>[335443.339458] sr_do_ioctl+0xe3/0x210 [sr_mod (HASH:ab3e 2)]
> >> <4>[335443.339471] sr_packet+0x3d/0x50 [sr_mod (HASH:ab3e 2)]
> >> <4>[335443.339482] cdrom_mrw_exit+0xc1/0x240 [cdrom (HASH:9d9a 3)]
> >> <4>[335443.339497] sr_free_disk+0x45/0x60 [sr_mod (HASH:ab3e 2)]
> >> <4>[335443.339506] disk_release+0xc8/0xe0
> >> <4>[335443.339515] device_release+0x39/0x90
> >> <4>[335443.339523] kobject_release+0x49/0xb0
> >> <4>[335443.339533] bdev_release+0x19/0x30
> >> <4>[335443.339540] deactivate_locked_super+0x3b/0x100
> >> <4>[335443.339548] cleanup_mnt+0xaa/0x160
> >> <4>[335443.339557] task_work_run+0x6c/0xb0
> >> <4>[335443.339563] exit_to_user_mode_prepare+0x102/0x120
> >> <4>[335443.339571] syscall_exit_to_user_mode+0x1a/0x30
> >> <4>[335443.339577] do_syscall_64+0x7e/0xa0
> >> <4>[335443.339582] ? exit_to_user_mode_prepare+0x44/0x120
> >> <4>[335443.339588] entry_SYSCALL_64_after_hwframe+0x55/0xbf
> >> <4>[335443.339595] RIP: 0033:0x7d52bea41f07
> 
> This just looks totally broken, the cdrom layer trying to issue block
> layer commands at exit time. Perhaps something like the below (utterly
> untested) patch would be an improvement. Also gets rid of the silly
> ->exit() hook which exists just for mrw.
> 
> 
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index 21a10552da61..31ba1f8c1f78 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -624,9 +624,6 @@ int register_cdrom(struct gendisk *disk, struct cdrom_device_info *cdi)
>  	if (check_media_type == 1)
>  		cdi->options |= (int) CDO_CHECK_TYPE;
>  
> -	if (CDROM_CAN(CDC_MRW_W))
> -		cdi->exit = cdrom_mrw_exit;
> -
>  	if (cdi->ops->read_cdda_bpc)
>  		cdi->cdda_method = CDDA_BPC_FULL;
>  	else
> @@ -651,9 +648,6 @@ void unregister_cdrom(struct cdrom_device_info *cdi)
>  	list_del(&cdi->list);
>  	mutex_unlock(&cdrom_mutex);
>  
> -	if (cdi->exit)
> -		cdi->exit(cdi);
> -
>  	cd_dbg(CD_REG_UNREG, "drive \"/dev/%s\" unregistered\n", cdi->name);
>  }
>  EXPORT_SYMBOL(unregister_cdrom);
> @@ -1264,6 +1258,8 @@ void cdrom_release(struct cdrom_device_info *cdi)
>  		cd_dbg(CD_CLOSE, "Use count for \"/dev/%s\" now zero\n",
>  		       cdi->name);
>  		cdrom_dvd_rw_close_write(cdi);
> +		if (CDROM_CAN(CDC_MRW_W))
> +			cdrom_mrw_exit(cdi);
>  
>  		if ((cdo->capability & CDC_LOCK) && !cdi->keeplocked) {
>  			cd_dbg(CD_CLOSE, "Unlocking door!\n");
> diff --git a/include/linux/cdrom.h b/include/linux/cdrom.h
> index fdfb61ccf55a..b907e6c2307d 100644
> --- a/include/linux/cdrom.h
> +++ b/include/linux/cdrom.h
> @@ -62,7 +62,6 @@ struct cdrom_device_info {
>  	__u8 last_sense;
>  	__u8 media_written;		/* dirty flag, DVD+RW bookkeeping */
>  	unsigned short mmc3_profile;	/* current MMC3 profile */
> -	int (*exit)(struct cdrom_device_info *);
>  	int mrw_mode_page;
>  	bool opened_for_data;
>  	__s64 last_media_change_ms;
> 
> -- 
> Jens Axboe

Happy to test. The patch makes a lot of sense. Will try and
recreate this as well - have just built 6.6 LTS and have it running in a
VM. Will report back. Many thanks.

Regards,
Phil