linux-kernel - Re: BUG: file descriptors leak when 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-next>] [day] [month] [year] [list]

Message-Id: <20071213132148.8f18a107.akpm@linux-foundation.org>
Date:	Thu, 13 Dec 2007 13:21:48 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	"Changli Gao" <xiaosuo@...il.com>
Cc:	linux-kernel@...r.kernel.org, Jens Axboe <jens.axboe@...cle.com>
Subject: Re: BUG: file descriptors leak when sys_pipe failed with -EFAULT

On Thu, 13 Dec 2007 20:34:11 +0800
"Changli Gao" <xiaosuo@...il.com> wrote:

> If an invalid address is passed to system call pipe as argument, file
> descriptors will leak.

Yup.  I added linux-kernel to cc.

> System call pipe is implemented as following on most architectures:
> 
>     int fd[2];
>     int error;
> 
>     error = do_pipe(fd);
>     if (!error) {
>         if (copy_to_user(fildes, fd, 2*sizeof(int)))
>             error = -EFAULT;
>     }
>     return error;
> 
> Invalid memory address makes copy_to_user failed. But the descriptors
> allocated for the pipe will be left open.
> A workaround fix will be like this:
> 
>     int fd[2];
>     int error;
> 
>     error = do_pipe(fd);
>     if (!error) {
>         if (copy_to_user(fildes, fd, 2*sizeof(int))) {
>             sys_close(fd[0]);
>             sys_close(fd[1]);
>             error = -EFAULT;
>         }
>     }
>     return error;
> 
> I don't understand the others architectures(such as
> sh/sh64/mips/sparc/sparc64)  which implement pipe in the other ways,
> so I just indicate this bug and provide my fixing way instead of
> patching it.

The consequences of this are that the application may eventually run out of
file descriptors and they will be cleaned up when the application exits
anyway, so it isn't terribly serious.

However it does seem fairly dumb of us to leave the fds open given that
at least one or possibly both of the file descriptors are unknown to the
application anyway.  Probably it'd be better to close them off immediately.

This would be an application-visible change: subsequent open()s will return
lower-numbered descriptors than they do at present.  That shouldn't matter.
--
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/