linux-kernel - Re: 2.6.27-rc7-sha1: EIP at proc_sys

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]

Date:	Fri, 26 Sep 2008 08:47:51 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Alexey Dobriyan <adobriyan@...il.com>
cc:	viro@...iv.linux.org.uk, ebiederm@...ssion.com,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: 2.6.27-rc7-sha1: EIP at proc_sys_compare+0x36/0x50



On Fri, 26 Sep 2008, Alexey Dobriyan wrote:
>
> Gentlemen, this happened while script was slowly rebuilding 300+ configs
> sequentially. Very little recompling activity itself, much seeking.
> 
> This is first time I see this. No debugging was on, no preemption.
> 
> Version: 2.6.27-rc7-c0f4d6d4b14a75a341d972ff73fb9740e1ceb634 +
> 	atl1 fixlet + "notes" kobject fixlet, but they don't matter.
> 
> ffffffff802bc690 <proc_sys_compare>:
....
> ffffffff802bc6c0:	75 dd                	jne    ffffffff802bc69f <proc_sys_compare+0xf>
> ffffffff802bc6c2:	49 8b 40 e0          	mov    -0x20(%r8),%rax
> ffffffff802bc6c6: ===>	48 8b 78 f0          	mov    -0x10(%rax),%rdi		<===
> ffffffff802bc6ca:	e8 71 96 f7 ff       	callq  ffffffff80235d40 <sysctl_is_seen>

That would be the

	sysctl_is_seen(PROC_I(dentry->d_inode)->sysctl)

call, and it really looks like 'dentry->d_inode' is NULL:

> [16526.029537] BUG: unable to handle kernel paging request at fffffffffffffff0

The whole PROC_I() thing just offsets from the inode:

	container_of(inode, struct proc_inode, vfs_inode);

and 'sysctl' is indeed 16 bytes below the vfs inode on x86-64:

	struct proc_inode {
		...
	        struct ctl_table_header *sysctl;
	        struct ctl_table *sysctl_entry;
	        struct inode vfs_inode;
	};

and as far as I can tell, there is nothing to say that a /proc inode 
cannot be a negative dentry. Sure, we try to get rid of them, but during a 
parallel lookup, we will have added the dentry with a NULL inode in the 
other lookup.

So assuming that you have an inode at that point seems to be utter crap.

Now, the whole _function_ is utter crap and should probably be dropped, 
but whatever. That's just another sysctl insanity. In the meantime, 
something like this does look appropriate, no?

Al, did I miss something?

		Linus
---
 fs/proc/proc_sysctl.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index f9a8b89..9435fd0 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -386,6 +386,8 @@ static int proc_sys_compare(struct dentry *dir, struct qstr *qstr,
 		return 1;
 	if (memcmp(qstr->name, name->name, name->len))
 		return 1;
+	if (!dentry->d_inode)
+		return 1;
 	return !sysctl_is_seen(PROC_I(dentry->d_inode)->sysctl);
 }
 
--
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/