linux-kernel - Re: [Regression] Crash in load

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: <alpine.LFD.2.00.1005251639420.3689@i5.linux-foundation.org>
Date:	Tue, 25 May 2010 16:47:32 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
cc:	Rusty Russell <rusty@...tcorp.com.au>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Brandon Philips <brandon@...p.org>
Subject: Re: [Regression] Crash in load_module() while freeing args

On Wed, 26 May 2010, Rafael J. Wysocki wrote:
>
> I'm not able to reproduce the issue with the following commit reverted:
> 
> commit 480b02df3aa9f07d1c7df0cd8be7a5ca73893455
> Author: Rusty Russell <rusty@...tcorp.com.au>
> Date:   Wed May 19 17:33:39 2010 -0600
> 
>     module: drop the lock while waiting for module to complete initialization.

Hmm. That does seem to be buggy. We can't just drop and re-take the lock: 
that may make sense _internally_ as far as resolve_symbol() itself is 
concerned, but the caller will its own local variables, and some of those 
will no longer be valid if the lock was dropped. 

That commit also changes the return value semantics of "use_module()", 
which is an exported interface where the only users seem to be 
out-of-kernel (the only in-kernel use is in kernel/module.c itself). That 
seems like a really really bad idea too.

So I think reverting it is definitely the right thing to do. The commit 
seems fundamentally broken. And having modules do request_module() in 
their init functions has always been invalid anyway, so that excuse 
doesn't really seem to be a reason to do anything crazy like this either.

Rewriting the logic to
 - not drop the lock
 - not change the return semantics of an exported interface
 - just make 'resolve_symbol()' fail if the module isn't fully loaded
would seem to be a more reasonable approach, no?

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