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

["Vegard Nossum" <vegard.nossum@gmail.com>]

On Tue, Nov 11, 2008 at 11:14 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> I queued the below for 2.6.28 inclusion and tagged for -stable
> backporting.
>
>
>
> From: Al Viro <viro@ZenIV.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@gmail.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