| Message ID | 20240204021739.1157830-1-viro@zeniv.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] fs/super.c: don't drop ->s_user_ns until we free struct super_block itself | expand |
On Sun, Feb 04, 2024 at 02:17:27AM +0000, Al Viro wrote: > Avoids fun races in RCU pathwalk... Same goes for freeing LSM shite > hanging off super_block's arse. > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- Hah, I once had the same patch for the userns bit because I was wondering about that, Reviewed-by: Christian Brauner <brauner@kernel.org> (Independent of whether or not this is pretty the s_user_ns should probably be a separate type so it can't be confused with other namespaces when checking permissions. Maybe I should respin my series for that if I find the time.)
On Sun, 2024-02-04 at 02:17 +0000, Al Viro wrote: > Avoids fun races in RCU pathwalk... Same goes for freeing LSM shite > hanging off super_block's arse. > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > fs/super.c | 13 ++++--------- > 1 file changed, 4 insertions(+), 9 deletions(-) > > diff --git a/fs/super.c b/fs/super.c > index d35e85295489..d6efeba0d0ce 100644 > --- a/fs/super.c > +++ b/fs/super.c > @@ -274,9 +274,10 @@ static void destroy_super_work(struct work_struct *work) > { > struct super_block *s = container_of(work, struct super_block, > destroy_work); > - int i; > - > - for (i = 0; i < SB_FREEZE_LEVELS; i++) > + security_sb_free(s); > + put_user_ns(s->s_user_ns); > + kfree(s->s_subtype); > + for (int i = 0; i < SB_FREEZE_LEVELS; i++) > percpu_free_rwsem(&s->s_writers.rw_sem[i]); nit: put_user_ns can call __put_user_ns which ends up queueing yet another workqueue job. It might be nice in the future to come up with a way to do the work that __put_user_ns does directly here instead of queueing it. OTOH, maybe it's not worth the effort... > kfree(s); > } > @@ -296,9 +297,6 @@ static void destroy_unused_super(struct super_block *s) > super_unlock_excl(s); > list_lru_destroy(&s->s_dentry_lru); > list_lru_destroy(&s->s_inode_lru); > - security_sb_free(s); > - put_user_ns(s->s_user_ns); > - kfree(s->s_subtype); > shrinker_free(s->s_shrink); > /* no delays needed */ > destroy_super_work(&s->destroy_work); > @@ -409,9 +407,6 @@ static void __put_super(struct super_block *s) > WARN_ON(s->s_dentry_lru.node); > WARN_ON(s->s_inode_lru.node); > WARN_ON(!list_empty(&s->s_mounts)); > - security_sb_free(s); > - put_user_ns(s->s_user_ns); > - kfree(s->s_subtype); > call_rcu(&s->rcu, destroy_super_rcu); > } > } Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/fs/super.c b/fs/super.c index d35e85295489..d6efeba0d0ce 100644 --- a/fs/super.c +++ b/fs/super.c @@ -274,9 +274,10 @@ static void destroy_super_work(struct work_struct *work) { struct super_block *s = container_of(work, struct super_block, destroy_work); - int i; - - for (i = 0; i < SB_FREEZE_LEVELS; i++) + security_sb_free(s); + put_user_ns(s->s_user_ns); + kfree(s->s_subtype); + for (int i = 0; i < SB_FREEZE_LEVELS; i++) percpu_free_rwsem(&s->s_writers.rw_sem[i]); kfree(s); } @@ -296,9 +297,6 @@ static void destroy_unused_super(struct super_block *s) super_unlock_excl(s); list_lru_destroy(&s->s_dentry_lru); list_lru_destroy(&s->s_inode_lru); - security_sb_free(s); - put_user_ns(s->s_user_ns); - kfree(s->s_subtype); shrinker_free(s->s_shrink); /* no delays needed */ destroy_super_work(&s->destroy_work); @@ -409,9 +407,6 @@ static void __put_super(struct super_block *s) WARN_ON(s->s_dentry_lru.node); WARN_ON(s->s_inode_lru.node); WARN_ON(!list_empty(&s->s_mounts)); - security_sb_free(s); - put_user_ns(s->s_user_ns); - kfree(s->s_subtype); call_rcu(&s->rcu, destroy_super_rcu); } }
Avoids fun races in RCU pathwalk... Same goes for freeing LSM shite hanging off super_block's arse. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- fs/super.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)