linux-kernel - Re: v2.6.28-rc1: readlink /proc/*/exe returns uninitialized data to userspace

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:	Tue, 11 Nov 2008 23:53:56 +0100
From:	"Vegard Nossum" <vegard.nossum@...il.com>
To:	"Andrew Morton" <akpm@...ux-foundation.org>
Cc:	"Alexey Dobriyan" <adobriyan@...il.com>, greg@...ah.com,
	mingo@...e.hu, viro@...iv.linux.org.uk, ebiederm@...ssion.com,
	Yoshiya.Koyama@...com, rjw@...k.pl, penberg@...helsinki.fi,
	linux-kernel@...r.kernel.org, kay.sievers@...y.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: v2.6.28-rc1: readlink /proc/*/exe returns uninitialized data to userspace

On Tue, Nov 11, 2008 at 11:14 PM, Andrew Morton
<akpm@...ux-foundation.org> wrote:
> I queued the below for 2.6.28 inclusion and tagged for -stable
> backporting.
>
>
>
> From: Al Viro <viro@...IV.linux.org.uk>
>
> Vegard sayeth:
>
> When I run readlink on the /proc/*/exe-file for udevd, the kernel
> returns some unitialized data to userspace:
>
> # strace -e trace=readlink readlink /proc/4762/exe
> readlink("/proc/4762/exe", "/sbin/udevd", 1025) = 30
>
> You can see it because the kernel thinks that the string is 30 bytes
> long, but in fact it is only 12 (including the '\0').
>
> If we explicitly clear the buffer before calling readlink, we can also
> see that some garbage has been filled in there, after the string:
>
> # ./readlink /proc/4762/exe
> readlink(/proc/4762/exe) = 30
> 2f7362696e2f7564657664000000ffffffad4effffffadffffffdeffffffffffffffff202864656c657465642900000000000000000000000000000
>
> (Output is from following simple program:)
>
> #include <stdio.h>
> #include <string.h>
> #include <unistd.h>
>
> int main(int argc, char *argv[])
> {
>        char buf[1024];

Should probably have been unsigned char.

>        int i;
>        ssize_t n;
>
>        memset(buf, 0, sizeof(buf));
>        n = readlink(argv[1], buf, sizeof(buf));
>
>        printf("readlink(%s) = %d\n", argv[1], n);
>
>        for (i = 0; i < sizeof(buf); ++i)
>                printf("%02x", buf[i]);

Or maybe it was the wrong format string. Negative numbers become very
long (with many extra "ff"s) in output. I guess it doesn't really
matter, though...

That said, Alexey also had an error in HIS testcase:

On Tue, Nov 4, 2008 at 11:34 AM, Alexey Dobriyan <adobriyan@...il.com> wrote:
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <stdio.h>
> #include <string.h>
> #include <unistd.h>
>
> int main(void)
> {
>        int fd, fd1;
>        char buf[64], buf1[64], img[42000];
>        ssize_t len;
>
>        fd = open("/proc/self/exe", O_RDONLY);
>
>        readlink("/proc/self/exe", buf, sizeof(buf));
>        strcpy(buf1, buf);
>        strcat(buf1, ".xxx");
>        unlink(buf1);
>        fd1 = open(buf1, O_WRONLY|O_CREAT);

Without the third argument to open with O_CREAT, file permissions may
be very strange :-)

Is this small program suitable for inclusion in LTP, maybe? We can
verify that kernel does the right thing by testing readlink(buf) ==
strlen(buf).

But thanks for the fix and attribution!


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--
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/