Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] cdrom: Fix race condition in cdrom_sysctl_register
To:     Guenter Roeck <linux@roeck-us.net>
Cc:     linux-kernel@vger.kernel.org
References: <1549516429-6542-1-git-send-email-linux@roeck-us.net>
From:   Jens Axboe <axboe@kernel.dk>
Message-ID: <7cd6ae8f-a29e-17e8-ddef-ee7c3ae45fb8@kernel.dk>
Date:   Fri, 8 Feb 2019 06:46:47 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <1549516429-6542-1-git-send-email-linux@roeck-us.net>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2/6/19 10:13 PM, Guenter Roeck wrote:
> The following traceback is sometimes seen when booting an image in qemu:
> 
> [   54.608293] cdrom: Uniform CD-ROM driver Revision: 3.20
> [   54.611085] Fusion MPT base driver 3.04.20
> [   54.611877] Copyright (c) 1999-2008 LSI Corporation
> [   54.616234] Fusion MPT SAS Host driver 3.04.20
> [   54.635139] sysctl duplicate entry: /dev/cdrom//info
> [   54.639578] CPU: 0 PID: 266 Comm: kworker/u4:5 Not tainted 5.0.0-rc5 #1
> [   54.639578] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
> [   54.641273] Workqueue: events_unbound async_run_entry_fn
> [   54.641273] Call Trace:
> [   54.641273]  dump_stack+0x67/0x90
> [   54.641273]  __register_sysctl_table+0x50b/0x570
> [   54.641273]  ? rcu_read_lock_sched_held+0x6f/0x80
> [   54.641273]  ? kmem_cache_alloc_trace+0x1c7/0x1f0
> [   54.646814]  __register_sysctl_paths+0x1c8/0x1f0
> [   54.646814]  cdrom_sysctl_register.part.7+0xc/0x5f
> [   54.646814]  register_cdrom.cold.24+0x2a/0x33
> [   54.646814]  sr_probe+0x4bd/0x580
> [   54.646814]  ? __driver_attach+0xd0/0xd0
> [   54.646814]  really_probe+0xd6/0x260
> [   54.646814]  ? __driver_attach+0xd0/0xd0
> [   54.646814]  driver_probe_device+0x4a/0xb0
> [   54.646814]  ? __driver_attach+0xd0/0xd0
> [   54.646814]  bus_for_each_drv+0x73/0xc0
> [   54.646814]  __device_attach+0xd6/0x130
> [   54.646814]  bus_probe_device+0x9a/0xb0
> [   54.646814]  device_add+0x40c/0x670
> [   54.646814]  ? __pm_runtime_resume+0x4f/0x80
> [   54.646814]  scsi_sysfs_add_sdev+0x81/0x290
> [   54.646814]  scsi_probe_and_add_lun+0x888/0xc00
> [   54.646814]  ? scsi_autopm_get_host+0x21/0x40
> [   54.646814]  __scsi_add_device+0x116/0x130
> [   54.646814]  ata_scsi_scan_host+0x93/0x1c0
> [   54.646814]  async_run_entry_fn+0x34/0x100
> [   54.646814]  process_one_work+0x237/0x5e0
> [   54.646814]  worker_thread+0x37/0x380
> [   54.646814]  ? rescuer_thread+0x360/0x360
> [   54.646814]  kthread+0x118/0x130
> [   54.646814]  ? kthread_create_on_node+0x60/0x60
> [   54.646814]  ret_from_fork+0x3a/0x50
> 
> The only sensible explanation is that cdrom_sysctl_register() is called
> twice, once from the module init function and once from register_cdrom().
> cdrom_sysctl_register() is not mutex protected and may happily execute
> twice if the second call is made before the first call is complete.
> 
> Use a static atomic to ensure that the function is executed exactly once.

Applied, thanks.

-- 
Jens Axboe