Discussion:
[PATCH 0/5] fuse: handle release synchronously (v4)
(too old to reply)
Al Viro
2014-10-18 18:22:41 UTC
Permalink
Raw Message
On Sat, Oct 18, 2014 at 8:35 AM, Linus Torvalds
Look around for AIO. Look around for the loop driver. Look around for
a number of things that do "fget()" and that you completely ignored.
.. actually, there are more instances of "get_file()" than of
"fget()", the aio one just happened to be the latter form. Lots and
lots of ways to get ahold of a file descriptor that keeps it open past
the "last close".
FWIW, procfs patch touches a very annoying issue: ->show_fdinfo() being
blocking. I would really like to get rid of that particular get_file()
and even more so - of get_files_struct() in there.

I certainly agree that anyone who expects that close() means the end of IO
is completely misguided. Mappings don't disappear on close(), neither does
a descriptor returned by dup(), or one that child got over fork(),
or something sent over in SCM_RIGHTS datagram, or, as you suggested, made
backing store for /dev/loop, etc.

What's more, in the example given upthread, somebody might've spotted that
file in /proc/<pid>/fd/* and *opened* it. At which point umount would
have to fail with EBUSY. And the same lsof(8) might've done just that.

It's not a matter of correctness or security, especially since somebody who
could do that, could've stopped your process, PTRACE_POKEd a fairly short
series of syscalls that would connect to AF_UNIX socket, send the file
over to them and clean after itself, then single-stepped through all of that,
restored the original state and resumed your process.

It is a QoI matter, though. And get_files_struct() in there is a lot more
annoying than get_file()/fput(). Suppose you catch the process during
exit(). All of a sudden, read from /proc/<pid>/fdinfo/<n> ends up doing
shitloads of filp_close(). It would be nice to avoid that.

Folks, how much pain would it be to make ->show_fdinfo() non-blocking?
Eric W. Biederman
2014-10-18 22:44:13 UTC
Permalink
Raw Message
Post by Al Viro
On Sat, Oct 18, 2014 at 8:35 AM, Linus Torvalds
Look around for AIO. Look around for the loop driver. Look around for
a number of things that do "fget()" and that you completely ignored.
.. actually, there are more instances of "get_file()" than of
"fget()", the aio one just happened to be the latter form. Lots and
lots of ways to get ahold of a file descriptor that keeps it open past
the "last close".
FWIW, procfs patch touches a very annoying issue: ->show_fdinfo() being
blocking. I would really like to get rid of that particular get_file()
and even more so - of get_files_struct() in there.
I certainly agree that anyone who expects that close() means the end of IO
is completely misguided. Mappings don't disappear on close(), neither does
a descriptor returned by dup(), or one that child got over fork(),
or something sent over in SCM_RIGHTS datagram, or, as you suggested, made
backing store for /dev/loop, etc.
What's more, in the example given upthread, somebody might've spotted that
file in /proc/<pid>/fd/* and *opened* it. At which point umount would
have to fail with EBUSY. And the same lsof(8) might've done just that.
It's not a matter of correctness or security, especially since somebody who
could do that, could've stopped your process, PTRACE_POKEd a fairly short
series of syscalls that would connect to AF_UNIX socket, send the file
over to them and clean after itself, then single-stepped through all of that,
restored the original state and resumed your process.
It is a QoI matter, though. And get_files_struct() in there is a lot more
annoying than get_file()/fput(). Suppose you catch the process during
exit(). All of a sudden, read from /proc/<pid>/fdinfo/<n> ends up doing
shitloads of filp_close(). It would be nice to avoid that.
Folks, how much pain would it be to make ->show_fdinfo() non-blocking?
I took a quick look and there are a couple of instances in tun,
eventpoll, and fanotify/inotify that take a spinlock while traversing
the data that needs to be printed.

So it would take a good hard stare at those pieces of code to understand
the locking, and potentially rewrite those routines.

The only one I am particularly familiar with tun did not look
fundamentally hard to change but it also isn't something I would
casually do either, as it would be easy to introduce nasty races by
accident.

Eric

Loading...