lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-id: <170242728484.12910.12134295135043081177@noble.neil.brown.name> Date: Wed, 13 Dec 2023 11:28:04 +1100 From: "NeilBrown" <neilb@...e.de> To: "Al Viro" <viro@...iv.linux.org.uk> Cc: "Chuck Lever" <chuck.lever@...cle.com>, "Christian Brauner" <brauner@...nel.org>, "Jens Axboe" <axboe@...nel.dk>, "Oleg Nesterov" <oleg@...hat.com>, "Jeff Layton" <jlayton@...nel.org>, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, linux-nfs@...r.kernel.org Subject: Re: [PATCH 1/3] nfsd: use __fput_sync() to avoid delayed closing of files. On Tue, 12 Dec 2023, Al Viro wrote: > On Mon, Dec 11, 2023 at 11:13:30PM +0000, Al Viro wrote: > > > dentry_kill() means ->d_release(), ->d_iput() and anything final iput() > > could do. Including e.g. anything that might be done by afs_silly_iput(), > > with its "send REMOVE to server, wait for completion". No, that's not > > a deadlock per se, but it can stall you a bit more than you would > > probably consider tolerable... Sure, you could argue that AFS ought to > > make that thing asynchronous, but... > > > > Anyway, it won't be "safe to use in most contexts". ->mmap_lock alone > > is enough for that, and that's just the one I remember to have given > > us a lot of headache. And that's without bringing the "nfsd won't > > touch those files" cases - make it generally accessible and you get > > to audit all locks that might be taken when we close a socket, etc. > > PS: put it that way - I can buy "nfsd is doing that only to regular > files and not on an arbitrary filesystem, at that; having the thread > wait on that sucker is not going to cause too much trouble"; I do *not* > buy turning it into a thing usable outside of a very narrow set of > circumstances. > Can you say more about "not on an arbitrary filesystem" ? I guess you means that procfs and/or sysfs might be problematic as may similar virtual filesystems (nfsd maybe). Could we encode some of this in the comment for __fput_sync ?? /** * __fput_sync : drop reference to a file synchronously * @f: file to drop * * Drop a reference on a file and do most cleanup work before returning. * * Due the the wide use of files in the design of Linux, dropping the * final reference to a file can result in dropping the final reference * to any of a large variety of other objects. Dropping those final * references can result in nearly arbitrary work. It should be assumed * that, unless prior checks or actions confirm otherwise, calling * __fput_sync() might: * - allocate memory * - perform synchronous IO * - wait for a remote service (for networked filesystems) * - take ->i_rwsem and other related VFS and filesystem locks * - take ->s_umount (if file is on a MNT_INTERNAL filesystem) * - take locks in a device driver if the file is CHR, BLK or SOCK * * If the caller cannot be confident that none of these will cause a * problem, it should use fput() instead. * * Note that the final unmount of a lazy-unmounted non-MNT_INTERNAL * filesystem will always be handled asynchronously. Individual drivers * might also leave some clean up to asynchronous threads. */ Thanks, NeilBrown
Powered by blists - more mailing lists