[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aHWLVhKKi0QAg9yM@equinox>
Date: Mon, 14 Jul 2025 23:57:26 +0100
From: Phillip Potter <phil@...lpotter.co.uk>
To: Jens Axboe <axboe@...nel.dk>
Cc: Phillip Potter <phil@...lpotter.co.uk>,
Sergey Senozhatsky <senozhatsky@...omium.org>,
Christoph Hellwig <hch@...radead.org>,
Chris Rankin <rankincj@...il.com>, linux-kernel@...r.kernel.org,
linux-block@...r.kernel.org
Subject: Re: cdrom: cdrom_mrw_exit() NULL ptr deref
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
Powered by blists - more mailing lists