linux-kernel - Re: [3.10] Oopses in kmem_cache_allocate() via prepare

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CA+55aFx8U4R9+wefpStwgXjTY+LoP0LJUa3eVCV2PK6MJWKBUw@mail.gmail.com>
Date:	Tue, 26 Nov 2013 15:44:26 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Simon Kirby <sim@...tway.ca>
Cc:	Ian Applegate <ia@...udflare.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Christoph Lameter <cl@...two.org>,
	Pekka Enberg <penberg@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Chris Mason <chris.mason@...ionio.com>
Subject: Re: [3.10] Oopses in kmem_cache_allocate() via prepare_creds()

On Tue, Nov 26, 2013 at 3:16 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> I'm really not very happy with the whole pipe locking logic (or the
> refcounting we do, separately from the "struct inode"), and in that
> sense I'm perfectly willing to blame that code for doing bad things.
> But the fact that it all goes away with debugging makes me very very
> unhappy.

Al, I really hate the "pipe_lock()" function that tests for
"pipe->files" being non-zero. I don't think there are any valid cases
where pipe->file ever *could* be zero, and if there are, they are
fundamentally racy.

Is there really any reason for that test? It used to test for
"pipe->inode" and that kind of made sense as the pipe didn't even have
a lock (it re-used the inode one). But these days that test makes zero
sense, except as a "don't even bother locking if this is some fake
internal pipe", but is that even valid?

Also, why does "pipe_release()" still use i_pipe. I'd much rather it
used "file->private_data" like everything else (and then it
unconditionally clear it). We are, after all, talking about releasing
the *file*, and we shouldn't be mixing up that inode in there.

IOW, what is wrong with the attached patch? Then we could/should

 - make the free_pipe_info() happen from the drop_inode()

 - delete pipe->files counter entirely because it has no valid use

Hmm?

                 Linus

View attachment "patch.diff" of type "text/plain" (1113 bytes)