Re: v2.6.28-rc1: readlink /proc/*/exe returns uninitialized datato userspace

[Al Viro]

On Tue, Nov 04, 2008 at 02:54:35AM -0800, Eric W. Biederman wrote:

> does a memcpy of the new name to the target name,
> but it doesn't do anything with the source name.
> Then later we swap the name lengths.
> 
> So the length on the dentry no longer matches the data
> we put in the buffer.

Aye.  BTW, here's yesterday IRC log, after eparis had pointed to
" (deleted)" in the ends of two more reproducers (auditd spew after
crond upgrade, FWIW - same issue with d_path()):

13:48 #sec-eng: < viro> eparis: it's switch_names()
13:49 #sec-eng: < viro> if both names are internal
13:49 #sec-eng: < viro> in that case we want to copy ->d_name.len instead of swapping those
13:50 #sec-eng: < viro> IOW, I understand what's going on; it's not too nasty, fortunately, but it needs fixing
13:51 #sec-eng: < viro> pathname ends on " (deleted)" and has sane path
13:51 #sec-eng: < viro> i.e. we have dentry->d_name.{name,len} responsible for everything in between
13:52 #sec-eng: < viro> and .len is more than it ought to be
13:52 #sec-eng: < viro> OTOH, it's still within the limit on name length, so it's embedded into struct dentry
13:53 #sec-eng: < viro> i.e. we had either unlink() doing something _very_ odd or rename() buggering the name/len for (unhashed) target
13:53 #sec-eng: < viro> rename() => d_move()
13:54 #sec-eng: < viro> then testing a bit with copying /bin/sh to /tmp/<different names>, starting those and doing mv(1) of one over another had happily reproduced it
13:55 #sec-eng: < viro> so that was d_move() with short-over-short and source name longer than target one
13:56 #sec-eng: < viro> since there are only two lines handling d_name there (
13:56 #sec-eng: < viro>         /* Switch the names.. */
13:56 #sec-eng: < viro>         switch_names(dentry, target);
13:56 #sec-eng: < viro>         do_switch(dentry->d_name.len, target->d_name.len);
13:57 #sec-eng: < viro> and switch_names() is 4-way choice by "is the source name short enough"/"is the target name short enough"...
13:58 #sec-eng: < viro> IOW, after noticing that " (deleted)" it had been absolutely straightforward
13:58 #sec-eng: < viro> it's not junk in the end
13:59 #sec-eng: < viro> it's junk in the _middle_

> Certainly not a resource leak or any kind of deadlock.
> And the length is right.  But it is an information leak.
> 
> I suppose a clever person could figure out how to steal
> information that way.

Not much, but that certainly needs fixing, leak or not.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/