linux-kernel - Re: [PATCH RFC] [PATCH] drivers/scsi/ch.c: Remove BKL in ch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20091011153709.4438cf9c@lxorguk.ukuu.org.uk>
Date:	Sun, 11 Oct 2009 15:37:09 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	John Kacur <jkacur@...hat.com>
Cc:	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...e.hu>,
	Christoph Hellwig <hch@...radead.org>,
	Jonathan Corbet <corbet@....net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Vincent Sanders <vince@...tec.co.uk>,
	linux-scsi@...r.kernel.org
Subject: Re: [PATCH RFC] [PATCH] drivers/scsi/ch.c: Remove BKL in ch_open

On Sun, 11 Oct 2009 15:45:16 +0200 (CEST)
John Kacur <jkacur@...hat.com> wrote:

[linux-scsi cc'd]

> Locking in ch_open is covered by the spin_lock, it serializes the calls
> to idr_find and scsi_device_get. The BKL appears redundant to me here.

I'm not so sure. In fact there are some quite umm interesting questions
about this code, and some of them are shared with other modules too.

Consider the following sequence

		CPU1				CPU2
	register_chrdev
		ok
						open device
						takes lock
	scsi_register_driver
		error

	unregister_chrdev
	error
	unload
						??????

We don't allocate any idr entries in open so the paths don't seem to leak
and the module itself looks correct. Looking at the bigger picture
however I am not sure what the module loader is trying to do in
kernel/module.c with the code at

     if (ret < 0) {
                /* Init routine failed: abort.  Try to protect us from
                   buggy refcounters. */
                mod->state = MODULE_STATE_GOING;
                synchronize_sched();
                module_put(mod);
                blocking_notifier_call_chain(&module_notify_list,

but it doesn't seem to be sufficient for what may go on ?

Rusty ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/