Re: [PATCH] thread wakeup fix for 2.4.0-test7

From: Alexander Viro (viro@math.psu.edu)
Date: Sat Aug 26 2000 - 14:00:57 EST


On Sat, 26 Aug 2000, Linus Torvalds wrote:

> On Sat, 26 Aug 2000, Rusty Russell wrote:
> >
> > > Why do you call this a bug?
> > >
> > > Thread 1 still has the fd open (it's used by the poll()).
> > >
> > > The fact that thread 2 removed it from the fd table is immaterial.
> >
> > Your implementation disagrees with you: poll already returns POLLNVAL.
> > With the patch it just returns it immediately, rather than returning
> > it when/if it times out.

Sigh... poll() API is _not_ thread-safe. It would make a lot of sense if
you would get _new_ (duplicate) descriptors as if you've received an
SCM_RIGHTS packet. The way it is defined you are getting an answer in
terms of descriptors you had when you had called poll(). Unless you've
done the protection in userland you are fscked, because they may have
completely different meanings when you return. Yes, it's asking for
trouble. Yes, inside the kernel such API would be fixed, just to avoid a
shitload of potential races. Unfortunately it's exported to userland, so
changing it is out of question.

> Tough. It's still because YOUR program is buggy. You closed a file
> descriptor that was in use.

Yup.

> The above is exactly what Linux does. The file descriptor is the _integer
> number_allocated_to_that_process_ to describe the file. It has got NOTHING
> to do with the file itself - think about dup() etc.

Not even to the process - to fdesc process group. I.e. descriptor-to-file
binding is a shared resource exposed to userland and should be treated as
such - with userland taking care about not stepping on its own toes.

> The _file_ is kept open by the poll(). The file descriptor is closed.
>
> Think about mmap(): you can do
>
> mmap(fd)
> close(fd);
> ... the _file_ is still active through the mapping ...
>
> and the mmap doesn't go away. Nobody expects the mmap() to go away just
> because you closed the fd. It's still there - and in fact SuS _requires_
> that it is still there.

Erm... Actually *BSD _does_ munmap() on close(). OTOH their handling of
shared descriptor tables is not just broken, it's dancing around with a
huge "screw me" sign on its arse (you can panic the box with UID==-2 and
_no_ permissions on any file or directory - having stdin opened is enough
to work with if you can call pipe(), rfork(), dup2() and close()).

> "fd" is just a handle. Once you've looked it up, it's not used any more.
> Closing it after the fact is a non-issue - you just got rid of the handle
> that we've already used.

Which is the reason why poll() turns out to be a dangerous thing in case
of shared file descriptor tables. Use with care. Kernel can't do anything
about that - if thread A call poll() and gets scheduled away immediately
after the return from the system call... just what could kernel do to stop
the thread B from closing one of descriptors and opening a new file?
Woops, A is thinking that descriptor 69 has something to read from, but
now 69 is not what it used to be when we returned from poll(). Same goes
for pipe(), open(), dup2(), socket() - whatever.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Aug 31 2000 - 21:00:18 EST