linux-kernel - Re: [PATCH] vfs: Fix potential circular locking through setxattr() and removexattr()

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240731185525.gr3pgswzdvoqla2g@quack3>
Date: Wed, 31 Jul 2024 20:55:25 +0200
From: Jan Kara <jack@...e.cz>
To: Matthew Wilcox <willy@...radead.org>
Cc: Jan Kara <jack@...e.cz>, Christian Brauner <brauner@...nel.org>,
	David Howells <dhowells@...hat.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Jeff Layton <jlayton@...nel.org>, Gao Xiang <xiang@...nel.org>,
	netfs@...ts.linux.dev, linux-erofs@...ts.ozlabs.org,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vfs: Fix potential circular locking through setxattr()
 and removexattr()

On Wed 31-07-24 19:27:43, Matthew Wilcox wrote:
> On Wed, Jul 31, 2024 at 08:16:57PM +0200, Jan Kara wrote:
> > To fix this, either we'd have to keep the lower cache filesystem private to
> > cachefiles (but I don't think that works with the usecases) or we have to
> > somehow untangle this mmap_lock knot. This "page fault does quite some fs
> > locking under mmap_lock" problem is not causing filesystems headaches for
> > the first time. I would *love* to be able to always drop mmap_lock in the
> > page fault handler, fill the data into the page cache and then retry the
> > fault (so that filemap_map_pages() would then handle the fault without
> > filesystem involvement). It would make many things in filesystem locking
> > simpler. As far as I'm checking there are now not that many places that
> > could not handle dropping of mmap_lock during fault (traditionally the
> > problem is with get_user_pages() / pin_user_pages() users). So maybe this
> > dream would be feasible after all.
> 
> The traditional problem was the array of VMAs which was removed in
> commit b2cac248191b -- if we dropped the mmap_lock, any previous
> entries in that array would become invalid.  Now that array is gone,
> do we have any remaining dependencies on the VMAs remaining valid?

So as far as I've checked the callers of get_user_pages() /
pin_user_pages() I didn't find any that fundamentally could not handle
dropping of mmap_lock. So at least for callers I've seen it was mostly
about teaching them to handle dropped mmap_lock, reacquire it and possibly
reestablish some state which could get invalidated after the mmap_lock got
dropped.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR