[PATCH] ev: fix epoll_init fd leak

[Marc Lehmann]

On Mon, Oct 31, 2011 at 11:10:56AM +0100, Ben Noordhuis <info at bnoordhuis.nl> wrote:
> > If epoll_create1 ever returns 0 you basically have a security issue in your
> > program, and it's indeed best to leave it alone.
> >
> > (fd 0 is always stdin, it's always in use and can never leak).
> 
> Sorry, but you're wrong

See POSIX if you don't believe me, unless you claim that is wrong too.

> (and that remark about it being a security issue is patent nonsense).

There are a number of exploits possible because one part of a program
prints error messages to standard file descriptors and another
accidentally closes it, or similar constellations.

Thats because these fds have standard behaviour attached to them - if
an unsafe file happens to be attached to it (imagine when libc opens
the shadow file etc.) you can get information leaks or unintended file
changes.

This is not theoretical, search cert archives and you will find them.

Closing standard fds is not a security issue in itself, but it invites
such problems and is never necessary or useful, so therefore is bad
programming practice. Consider it in the same class as buffer overflows.

It should not be encouraged.

> Closing file descriptors 0-2 is a common practice for daemons.

Name one? For starters, please note that none of the daemons on my debian
system I work on does that (that includes sshd, dbus, ntpd, memcached,
automount, idmapd, mysql, cron...), which is evidence enough that it's not
common practise.

The reason it's not common practise is that it's almost always a bug and a
potential security issue, and is therefore bad programming practise.

If you do it, better fix it - common practise for daemons is to tie fds 0,
1, 2 to /dev/null or another "safe" file, such as some pipe fd.

On Mon, Oct 31, 2011 at 12:25:13PM +0100, Christian Parpart <trapni at gentoo.org> wrote:
> there are processes, that spawn other processes and first close all
> inherited file descriptors, including 0, 1, and 2,

Name one that is in debian or another linux distro? Even if you can (I
wouldn't be totally surprised), these cases should be exceedingly rare and
are almost certainly the result of bad programming.

> because the to be spawned child process is not meant to have any
> stdin/stdout/stderr at all (think of daemons).

These programs are badly written in any case, and are often the source for
local exploits (and rarely remote ones). They should be fixed.

> stick on conventions like always checking for < 0 on error instead of <= 0,
> as, in fact, epoll_create returns -1 (< 0) on error, and never 0, as 0 is -
> in fact - a valid file descriptor.

0 is, in fact, not a valid file descriptor in this case, as it is stdin,
and an epoll fd is not having the properties stdin has - althoguh it'S
much betterto have an epoll fd there than, say, your shadow file :)

-- 
                The choice of a       Deliantra, the free code+content MORPG
      -----==-     _GNU_              http://www.deliantra.net
      ----==-- _       generation
      ---==---(_)__  __ ____  __      Marc Lehmann
      --==---/ / _ \/ // /\ \/ /      schmorp at schmorp.de
      -=====/_/_//_/\_,_/ /_/\_\