On Tue 13-10-26 21:25:55, Christian Brauner wrote:
> On Tue, Oct 13, 2026 at 09:23:07PM +0200, Christian Brauner wrote:
> > On Tue, Oct 13, 2026 at 12:47:58PM +0200, Jan Kara wrote:
> > > 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
> >
> > So I prototyped your thing and all version I could see working end up
> > penalizing umount and filesystem shutdown quite heavily. Maybe I'm not
> > imagnative enough but everything involving synchronize_rcu* should be
> > kicked in the n*ts. Because if you're going onto big machines your
> > workloads will get said kick.
Thanks for trying! I kind of expected that it won't be acceptable after
reading all those changelogs.
> And one thing I forgot to say: that requirement exists today. It is
> entirely accidental that this wasn't an issue in the umount() path
> because of the synchronize_rcu* in there. But said grace period doesn't
> exist if you e.g., chdir() out of a mount and put the last reference
> while still in lookup. Then said filesystems are fscked today because
> that is a requirement. And various reproducers in this series illustrate
> that.
Yeah, I understood that from your description. The RCU grace period delay
would have to be somehow added to those cases as well.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
