Re: Xinetd /tmp race?

[glynn () SENSEI CO UK (Glynn Clements)]

Gigi Sullivan wrote:

> > > If you send SIGHUP to xinetd, you get a dump file to /tmp/xinetd.dump, but
> > > this method isn't checked against /tmp, and it happily overwrites anything
> > > in the place of that file.  The package has been released in 1997, IMHO this
> > > is too old to have a bug of this kind hidden.
> > >
> > > BTW here's the patch:
> >
> > [stat() before open() patch]
> >
> > 1. This suffers from a race condition (in fact, this is the textbook
> > example of a race condition). You need to fstat() the open()ed file,
> > and check that it's the same file that you just stat()ed.
> >
> > 2. The stat() needs to be an lstat(), to allow for symlinks.
>
> Uhm it shouldn't be enough to do the open() call with O_EXCL flag too ?

Two points:

1. I don't think that this is safe over NFS.

The Linux open(2) manpage says:

       O_EXCL When used with O_CREAT, if the file already  exists
              it  is  an error and the open will fail.  O_EXCL is
              broken on NFS file systems, programs which rely  on
              it for performing locking tasks will contain a race
              condition.  The solution for performing atomic file
              locking using a lockfile is to create a unique file
              on the same fs (e.g.,  incorporating  hostname  and
              pid),  use  link(2)  to make a link to the lockfile
              and use stat(2) on the unique file to check if  its
              link  count  has  increased  to  2.  Do not use the
              return value of the link() call.

2. The current behaviour is to append to an existing file if it
exists, or to create a new file if it doesn't. Adding O_EXCL would
break this behaviour.

I believe that the correct approach (in general) is to lstat() the
file, check that it's suitable (e.g. not a symlink), open() it,
fstat() the descriptor, and compare the device/inode pair with those
from the lstat(), to check that you did actually open the file which
you thought that you were opening.

IMHO, a better approach in this case would be to use a directory which
isn't world-writable e.g. /var/run, /var/log etc.

--
Glynn Clements <glynn () sensei co uk>