linux-kernel - Re: [PATCH] kill empty chardev open/release methods

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: <20080516215517.30101a39@core>
Date:	Fri, 16 May 2008 21:55:17 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Jonathan Corbet <corbet@....net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	Alexander Viro <viro@....linux.org.uk>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kill empty chardev open/release methods

> > If they literaly are 'return 0' you can just remove them, as a
> > non-existing open op will just be fine.
> 
> And here's a patch to do just that:  remove all empty chardev
> open/release methods.  Based on the list compiled by Jonathan.

Actually it turns out you can introduce bugs doing this when the BKL is
pushed down.

The problem is the methods are not NULL, they (with the lock pushed down
are)

{
	lock_kernel();
	unlock_kernel();
}

And we have drivers with setup code that does things in the wrong order
but under the BKL. eg one I just fixed did

	misc_register()
	init locks
	allocate memory
	do stuff
	return 0;

The lock/unlock in the open happens to save your butt against the wrong
order of intialisation because the open cannot occur before the lock is
taken, and thanks to the BKL it cannot make any progress until the setup
is completed. Fun too - udev loves opening things as they appear so in
some cases we might actually trigger them too.

So when you remove the _open() empty methods *please* make sure you have
verified the correctness and ordering of the entire registration path.
I've found three examples of this so far just cleaning up
drivers/watchdog.

Alan
--
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/