the mystery that is sock_fasync

the mystery that is sock_fasync

[David Miller]

I was studying sock_fasync() and it definitely has a bunch
of questionable issues.

Well firstly, it duplicates fasync_helper() entirely.
The only difference is that sock_fasync() does socket
local locking which is better for performance.  fasync_helper()
uses a global spinlock to protect the fasync list it is given.

Secondly, and I think more importantly, this thing acts as
if it is possible to have more than one file --> socket
mapping.  That is simply impossible.

There can indeed be many file descriptors that point to the
file object that points to the socket inode, but that's
different.

This invariant is maintained by the fact that socket
creations creates and maps one file object to point
to the socket's inode in sock_create.

Furthermore we block any attempt to open sockets by name
via things like /proc/$PID/fds/$sock_fdnum

In fact when sock_close() runs, it calls sock_fasync(-1, file, 0) and
the subsequent sock_release() bug checks that fasync_list is NULL.

If my analysis is correct we can incredibly simplify sock_fasync().

Did I miss some way that multiple file objects can point to the
same socket inode?

Re: the mystery that is sock_fasync

[Evgeniy Polyakov]

On Fri, Aug 11, 2006 at 03:15:16AM -0700, David Miller (davem@davemloft.net) wrote:
> Did I miss some way that multiple file objects can point to the
> same socket inode?

What about dup and pipe?

-- 
	Evgeniy Polyakov

Re: the mystery that is sock_fasync

[David Miller]

From: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Date: Fri, 11 Aug 2006 14:28:20 +0400

> On Fri, Aug 11, 2006 at 03:15:16AM -0700, David Miller (davem@davemloft.net) wrote:
> > Did I miss some way that multiple file objects can point to the
> > same socket inode?
> 
> What about dup and pipe?

Dup makes new "file descriptor" references to the file object.

It does not create a new file object reference to a socket inode,
which is what we're concerned with here.

Pipe files do not point to socket inodes.

Re: the mystery that is sock_fasync

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Fri, 11 Aug 2006 03:15:16 -0700 (PDT)

> If my analysis is correct we can incredibly simplify sock_fasync().

I've also found more bugs in sock_fasync(), it's a real can of worms
:-)

It deviates from the return value policies used by othe
file_ops->fasync() implementations.  For example, if we look at
fasync_helper() it clearly shows that we should return 1 if a list
manipulation (insert of delete) happened else it should return 0.

Also, my theory about struct file<-->struct socket being a one-to-one
mapping is fully supported by the fact that struct socket has a "file"
member that can only take on one value.

Re: the mystery that is sock_fasync

[Alexey Kuznetsov]

Hello!

> Did I miss some way that multiple file objects can point to the
> same socket inode?

Absolutely prohibited. Always was.

Apparently, sock_fasync() was cloned from tty_fasync(), that's the only
reason why it is so creepy.

Alexey