From: ebiederm@xmission.com (Eric W. Biederman)
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        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
References: <20140930191933.GC5011@tucsk.piliscsaba.szeredi.hu>
	<CA+55aFyD2Q6tJzRUbNyNFRdbSN09SAN9CqC5BTmCtOu4W9PKGw@mail.gmail.com>
	<542BE551.1010705@parallels.com>
	<CAJfpegs4Up_feUbqEGxfhq+rZneTmy0qcZy-k3so7VjufQY7-w@mail.gmail.com>
	<543F9E75.2090509@parallels.com>
	<CAJfpegvS+rmFqGgvHq183Z-MxLAwcgEB57LQxAwy3QAe5CaVwg@mail.gmail.com>
	<CA+55aFw68DMazGSu=YaOVVEWp=_LrD+2abah+5Tq3kpmH-r+Hg@mail.gmail.com>
	<20141017085509.GE5011@tucsk.piliscsaba.szeredi.hu>
	<CA+55aFyALiUbRVcxGGG79N-97pa997v9En64O9jLLBvBAO6vrA@mail.gmail.com>
	<CA+55aFyU2NgXYZSnNBwMR_ZyFuKzHna2NBBhwd6iev0uaEdqKA@mail.gmail.com>
	<20141018182241.GE7996@ZenIV.linux.org.uk>
Date: Sat, 18 Oct 2014 15:44:13 -0700
In-Reply-To: <20141018182241.GE7996@ZenIV.linux.org.uk> (Al Viro's message of
	"Sat, 18 Oct 2014 19:22:41 +0100")
Message-ID: <87a94t577m.fsf@x220.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH 0/5] fuse: handle release synchronously (v4)
Sender: linux-kernel-owner@vger.kernel.org

Al Viro <viro@ZenIV.linux.org.uk> writes:

> 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?

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/