public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	Maxim Patlasov <mpatlasov@parallels.com>,
	Anand Avati <avati@gluster.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Michael j Theall <mtheall@us.ibm.com>,
	fuse-devel <fuse-devel@lists.sourceforge.net>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 0/5] fuse: handle release synchronously (v4)
Date: Sat, 18 Oct 2014 19:22:41 +0100	[thread overview]
Message-ID: <20141018182241.GE7996@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFyU2NgXYZSnNBwMR_ZyFuKzHna2NBBhwd6iev0uaEdqKA@mail.gmail.com>

On Sat, Oct 18, 2014 at 08:40:05AM -0700, Linus Torvalds wrote:
> On Sat, Oct 18, 2014 at 8:35 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > 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?

  parent reply	other threads:[~2014-10-18 18:22 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-25 12:05 [PATCH 0/5] fuse: handle release synchronously (v4) Maxim Patlasov
2014-09-25 12:05 ` [PATCH 1/5] fuse: add FOPEN_SYNC_RELEASE flag to ff->open_flags Maxim Patlasov
2014-09-25 12:05 ` [PATCH 2/5] fuse: cosmetic rework of fuse_send_readpages Maxim Patlasov
2014-09-25 12:06 ` [PATCH 3/5] fuse: wait for end of IO on release Maxim Patlasov
2014-09-25 12:06 ` [PATCH 4/5] fuse: add mount option to disable synchronous release Maxim Patlasov
2014-09-25 12:07 ` [PATCH 5/5] fuse: enable close_wait " Maxim Patlasov
2014-09-26 15:28 ` [PATCH 0/5] fuse: handle release synchronously (v4) Miklos Szeredi
2014-09-30  3:15   ` Miklos Szeredi
2014-09-30  3:55 ` Linus Torvalds
     [not found]   ` <CAFboF2yhGyjk4e_CHQV5b2WvB-QhsWNyHvFiFG_OM_=3-KArLQ@mail.gmail.com>
2014-09-30  7:43     ` Miklos Szeredi
2014-09-30 18:22     ` Linus Torvalds
2014-09-30 19:04     ` Linus Torvalds
2014-09-30 19:19       ` Miklos Szeredi
2014-09-30 20:44         ` Linus Torvalds
2014-10-01  3:47           ` Miklos Szeredi
2014-10-01 11:28           ` Maxim Patlasov
2014-10-09  8:14             ` Miklos Szeredi
2014-10-16 10:31               ` Maxim Patlasov
2014-10-16 13:43                 ` Miklos Szeredi
2014-10-16 13:54                   ` Linus Torvalds
2014-10-17  8:55                     ` Miklos Szeredi
2014-10-18 15:35                       ` Linus Torvalds
2014-10-18 15:40                         ` Linus Torvalds
2014-10-18 18:01                           ` Miklos Szeredi
2014-10-18 18:24                             ` Al Viro
2014-10-18 18:45                               ` Al Viro
2014-10-18 18:38                             ` Linus Torvalds
2014-10-18 18:22                           ` Al Viro [this message]
2014-10-18 22:44                             ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141018182241.GE7996@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=avati@gluster.org \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mpatlasov@parallels.com \
    --cc=mtheall@us.ibm.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox