On Mon 12-10-26 12:42:06, Christian Brauner wrote:
> On Mon, Oct 12, 2026 at 12:36:15PM +0200, Jan Kara wrote:
> > On Fri 09-10-26 23:51:50, Christian Brauner wrote:
> > > While working on eliding the grace period in namespace_unlock() I went
> > > through what filesystems free when they're shut down and what a pathwalk
> > > in rcu mode can still read at that point. Since 66a97c2ec953's rcu
> > > pathwalk fixes that's the filesystem's job. Whatever ->permission(),
> > > ->d_revalidate(), ->get_link(), ->d_hash() and ->d_compare() look at in
> > > rcu mode has to be freed after a grace period. A few filesystems don't
> > > do that.
> > > 
> > > Most of it is hidden by umount(2). namespace_unlock() waits for a grace
> > > period before it drops the mounts, so a walker that was inside the
> > > filesystem is gone by the time the filesystem is shut down.
> > > 
> > > But the last reference to a mount doesn't have to go away in umount(2).
> > > A chdir() out of a lazily unmounted directory or the close() of the last
> > > file on it drops it without a grace period. A walker can then still be
> > > in the filesystem while ->put_super() and ->kill_sb() free what it
> > > reads.
> > 
> > Good catches! But I find it very counter-intuitive that pathname lookups
> > can still be running against a filesystem while its ->put_super is called.
> > People will keep forgetting about that and as a result these UAF bugs will
> > be introduced again. Isn't it feasible we'd guarantee to the filesystem
> > ->put_super() gets called only after all lookups are done (i.e., after RCU
> > grace period expires)?
> > 
> > I understand this is a hairy bussiness because this has userspace visible
> > implications (like when filesystem's sysfs files disappear, when the bdev
> > really gets unused, etc.) but keeping all filesystems safe to run RCU
> > lookup after ->put_super() will be IMHO a headache.
> 
> I don't think delaying put_super() or freeing the superblock helps.
> That's not where the issue is. The issue is with sb->s_fs_info being
> available after a grace period and that can go away in different
> locations ->put_super() or ->kill_sb().

So my thinking was: Cannot we separate the teardown of mounts and the
teardown of the sb by an RCU grace period? Then by the time the filesystem
gets to any freeing of filesystem related structures all RCU lookups are
guaranteed to be done so there's no problem with them accessing possibly
freed structures pointed to by the sb.

But then I've read more of the changelogs and AFAIU getting rid of the RCU
grace period waiting in the unmount path is actually your point to speedup
container teardown. I still think in-progress RCU lookups on a sb being
unmounted will be a source of bugs in the future (dependencies during fs
unmount are complex as is and adding RCU lookups to the game adds even more
complexity). Anyway, if we're going to do that then eventually I think
we'll need to add something like lockdep context tracking to verify each
structure accessed from RCU lookup is properly RCU-freed but that's for
later.

> Bringing put_super() into the argument was maybe not my best idea. We
> also need to backport this stuff.

OK, let's go with these fixes for now.

                                                                Honza

-- 
Jan Kara <[email protected]>
SUSE Labs, CR

Reply via email to