nilfs_find_or_create_root() publishes a new root in the checkpoint
tree before creating its sysfs object.  If sysfs registration fails,
the root is freed while it is still reachable from the tree.  A
concurrent nilfs_lookup_root() can then dereference freed memory.

The fix needs the lock to be held across the sysfs call, but
nilfs_sysfs_create_snapshot_group() can sleep, so the existing
spinlock is not suitable.

Convert ns_cptree_lock from a spinlock to a mutex.  All existing
callers are in process context (mount, lookup, segctor, recovery),
and nilfs_put_root() can use refcount_dec_and_mutex_lock() as the
atomic decrement-and-acquire primitive.

With the mutex, nilfs_find_or_create_root() can hold it across the
sysfs registration and only insert the root into the rbtree after
sysfs succeeds.  On failure, the root was never visible and can be
freed after waiting for the kobject release callback to complete.

Both the creation error path and the normal removal path must wait
for the embedded kobject release via wait_for_completion() before
freeing the container, because kobject_put() does not guarantee
synchronous release (CONFIG_DEBUG_KOBJECT_RELEASE defers it).

Fixes: dd70edbde262 ("nilfs2: integrate sysfs support into driver")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---

Changes in v3:
  - Move wait_for_completion() after mutex_unlock() in
    nilfs_put_root() to avoid holding the lock during an
    unbounded wait, as noted by Viacheslav Dubeyko.
    After rb_erase() the root is no longer in the tree
    and the refcount is zero, so no concurrent path can
    reach it.

Changes in v2:
  - Convert ns_cptree_lock from spinlock to mutex instead of adding
    a second lock, as suggested by Viacheslav Dubeyko.  Use
    refcount_dec_and_mutex_lock() in nilfs_put_root().

 fs/nilfs2/the_nilfs.c | 37 ++++++++++++++++++++-----------------
 fs/nilfs2/the_nilfs.h |  2 +-
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index 7b23e373a1..XXXXXXX 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -70,7 +70,7 @@ struct the_nilfs *alloc_nilfs(struct super_block *sb)
 	spin_lock_init(&nilfs->ns_inode_lock);
 	spin_lock_init(&nilfs->ns_last_segment_lock);
 	nilfs->ns_cptree = RB_ROOT;
-	spin_lock_init(&nilfs->ns_cptree_lock);
+	mutex_init(&nilfs->ns_cptree_lock);
 	init_rwsem(&nilfs->ns_segctor_sem);
 	nilfs->ns_sb_update_freq = NILFS_SB_FREQ;

@@ -846,7 +846,7 @@ struct nilfs_root *nilfs_lookup_root(struct the_nilfs *nilfs, __u64 cno)
 	struct rb_node *n;
 	struct nilfs_root *root;

-	spin_lock(&nilfs->ns_cptree_lock);
+	mutex_lock(&nilfs->ns_cptree_lock);
 	n = nilfs->ns_cptree.rb_node;
 	while (n) {
 		root = rb_entry(n, struct nilfs_root, rb_node);
@@ -857,11 +857,11 @@ struct nilfs_root *nilfs_lookup_root(struct the_nilfs *nilfs, __u64 cno)
 			n = n->rb_right;
 		} else {
 			refcount_inc(&root->count);
-			spin_unlock(&nilfs->ns_cptree_lock);
+			mutex_unlock(&nilfs->ns_cptree_lock);
 			return root;
 		}
 	}
-	spin_unlock(&nilfs->ns_cptree_lock);
+	mutex_unlock(&nilfs->ns_cptree_lock);

 	return NULL;
 }
@@ -881,7 +881,7 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
 	if (!new)
 		return NULL;

-	spin_lock(&nilfs->ns_cptree_lock);
+	mutex_lock(&nilfs->ns_cptree_lock);

 	p = &nilfs->ns_cptree.rb_node;
 	parent = NULL;
@@ -896,7 +896,7 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
 			p = &(*p)->rb_right;
 		} else {
 			refcount_inc(&root->count);
-			spin_unlock(&nilfs->ns_cptree_lock);
+			mutex_unlock(&nilfs->ns_cptree_lock);
 			kfree(new);
 			return root;
 		}
@@ -909,17 +909,19 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
 	atomic64_set(&new->inodes_count, 0);
 	atomic64_set(&new->blocks_count, 0);

-	rb_link_node(&new->rb_node, parent, p);
-	rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
-
-	spin_unlock(&nilfs->ns_cptree_lock);
-
 	err = nilfs_sysfs_create_snapshot_group(new);
 	if (err) {
+		mutex_unlock(&nilfs->ns_cptree_lock);
+		wait_for_completion(&new->snapshot_kobj_unregister);
 		kfree(new);
-		new = NULL;
+		return NULL;
 	}

+	rb_link_node(&new->rb_node, parent, p);
+	rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
+
+	mutex_unlock(&nilfs->ns_cptree_lock);
+
 	return new;
 }

@@ -927,14 +929,15 @@ void nilfs_put_root(struct nilfs_root *root)
 {
 	struct the_nilfs *nilfs = root->nilfs;

-	if (refcount_dec_and_lock(&root->count, &nilfs->ns_cptree_lock)) {
+	if (refcount_dec_and_mutex_lock(&root->count,
+					&nilfs->ns_cptree_lock)) {
 		rb_erase(&root->rb_node, &nilfs->ns_cptree);
-		spin_unlock(&nilfs->ns_cptree_lock);
-
 		nilfs_sysfs_delete_snapshot_group(root);
-		iput(root->ifile);
+		mutex_unlock(&nilfs->ns_cptree_lock);

+		wait_for_completion(&root->snapshot_kobj_unregister);
+		iput(root->ifile);
 		kfree(root);
 	}
 }
diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
index 4776a70f01..074644c64a 100644
--- a/fs/nilfs2/the_nilfs.h
+++ b/fs/nilfs2/the_nilfs.h
@@ -150,7 +150,7 @@ struct the_nilfs {

 	/* Checkpoint tree */
 	struct rb_root		ns_cptree;
-	spinlock_t		ns_cptree_lock;
+	struct mutex		ns_cptree_lock; /* Protects ns_cptree */

 	/* Dirty inode list */
 	struct list_head	ns_dirty_files;
--
2.43.0

On Wed, 2026-09-16 at 20:50 -0300, Aldo Ariel Panzardo wrote:
> nilfs_find_or_create_root() publishes a new root in the checkpoint
> tree before creating its sysfs object.  If sysfs registration fails,
> the root is freed while it is still reachable from the tree.  A
> concurrent nilfs_lookup_root() can then dereference freed memory.
> 
> The fix needs the lock to be held across the sysfs call, but
> nilfs_sysfs_create_snapshot_group() can sleep, so the existing
> spinlock is not suitable.
> 
> Convert ns_cptree_lock from a spinlock to a mutex.  All existing
> callers are in process context (mount, lookup, segctor, recovery),
> and nilfs_put_root() can use refcount_dec_and_mutex_lock() as the
> atomic decrement-and-acquire primitive.
> 
> With the mutex, nilfs_find_or_create_root() can hold it across the
> sysfs registration and only insert the root into the rbtree after
> sysfs succeeds.  On failure, the root was never visible and can be
> freed after waiting for the kobject release callback to complete.
> 
> Both the creation error path and the normal removal path must wait
> for the embedded kobject release via wait_for_completion() before
> freeing the container, because kobject_put() does not guarantee
> synchronous release (CONFIG_DEBUG_KOBJECT_RELEASE defers it).
> 
> Fixes: dd70edbde262 ("nilfs2: integrate sysfs support into driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
> 
> Changes in v3:
>   - Move wait_for_completion() after mutex_unlock() in
>     nilfs_put_root() to avoid holding the lock during an
>     unbounded wait, as noted by Viacheslav Dubeyko.
>     After rb_erase() the root is no longer in the tree
>     and the refcount is zero, so no concurrent path can
>     reach it.
> 
> Changes in v2:
>   - Convert ns_cptree_lock from spinlock to mutex instead of adding
>     a second lock, as suggested by Viacheslav Dubeyko.  Use
>     refcount_dec_and_mutex_lock() in nilfs_put_root().
> 
>  fs/nilfs2/the_nilfs.c | 37 ++++++++++++++++++++-----------------
>  fs/nilfs2/the_nilfs.h |  2 +-
>  2 files changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> index 7b23e373a1..XXXXXXX 100644
> --- a/fs/nilfs2/the_nilfs.c
> +++ b/fs/nilfs2/the_nilfs.c
> @@ -70,7 +70,7 @@ struct the_nilfs *alloc_nilfs(struct super_block
> *sb)
>  	spin_lock_init(&nilfs->ns_inode_lock);
>  	spin_lock_init(&nilfs->ns_last_segment_lock);
>  	nilfs->ns_cptree = RB_ROOT;
> -	spin_lock_init(&nilfs->ns_cptree_lock);
> +	mutex_init(&nilfs->ns_cptree_lock);
>  	init_rwsem(&nilfs->ns_segctor_sem);
>  	nilfs->ns_sb_update_freq = NILFS_SB_FREQ;
> 
> @@ -846,7 +846,7 @@ struct nilfs_root *nilfs_lookup_root(struct
> the_nilfs *nilfs, __u64 cno)
>  	struct rb_node *n;
>  	struct nilfs_root *root;
> 
> -	spin_lock(&nilfs->ns_cptree_lock);
> +	mutex_lock(&nilfs->ns_cptree_lock);
>  	n = nilfs->ns_cptree.rb_node;
>  	while (n) {
>  		root = rb_entry(n, struct nilfs_root, rb_node);
> @@ -857,11 +857,11 @@ struct nilfs_root *nilfs_lookup_root(struct
> the_nilfs *nilfs, __u64 cno)
>  			n = n->rb_right;
>  		} else {
>  			refcount_inc(&root->count);
> -			spin_unlock(&nilfs->ns_cptree_lock);
> +			mutex_unlock(&nilfs->ns_cptree_lock);
>  			return root;
>  		}
>  	}
> -	spin_unlock(&nilfs->ns_cptree_lock);
> +	mutex_unlock(&nilfs->ns_cptree_lock);
> 
>  	return NULL;
>  }
> @@ -881,7 +881,7 @@ nilfs_find_or_create_root(struct the_nilfs
> *nilfs, __u64 cno)
>  	if (!new)
>  		return NULL;
> 
> -	spin_lock(&nilfs->ns_cptree_lock);
> +	mutex_lock(&nilfs->ns_cptree_lock);
> 
>  	p = &nilfs->ns_cptree.rb_node;
>  	parent = NULL;
> @@ -896,7 +896,7 @@ nilfs_find_or_create_root(struct the_nilfs
> *nilfs, __u64 cno)
>  			p = &(*p)->rb_right;
>  		} else {
>  			refcount_inc(&root->count);
> -			spin_unlock(&nilfs->ns_cptree_lock);
> +			mutex_unlock(&nilfs->ns_cptree_lock);
>  			kfree(new);
>  			return root;
>  		}
> @@ -909,17 +909,19 @@ nilfs_find_or_create_root(struct the_nilfs
> *nilfs, __u64 cno)
>  	atomic64_set(&new->inodes_count, 0);
>  	atomic64_set(&new->blocks_count, 0);
> 
> -	rb_link_node(&new->rb_node, parent, p);
> -	rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
> -
> -	spin_unlock(&nilfs->ns_cptree_lock);
> -
>  	err = nilfs_sysfs_create_snapshot_group(new);
>  	if (err) {
> +		mutex_unlock(&nilfs->ns_cptree_lock);
> +		wait_for_completion(&new->snapshot_kobj_unregister);

Should we consider wait_for_completion_killable_timeout()? What do you
think? Do we need to use timeout?

>  		kfree(new);
> -		new = NULL;
> +		return NULL;
>  	}
> 
> +	rb_link_node(&new->rb_node, parent, p);
> +	rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
> +
> +	mutex_unlock(&nilfs->ns_cptree_lock);
> +
>  	return new;
>  }
> 
> @@ -927,14 +929,15 @@ void nilfs_put_root(struct nilfs_root *root)
>  {
>  	struct the_nilfs *nilfs = root->nilfs;
> 
> -	if (refcount_dec_and_lock(&root->count, &nilfs-
> >ns_cptree_lock)) {
> +	if (refcount_dec_and_mutex_lock(&root->count,
> +					&nilfs->ns_cptree_lock)) {
>  		rb_erase(&root->rb_node, &nilfs->ns_cptree);
> -		spin_unlock(&nilfs->ns_cptree_lock);
> -
>  		nilfs_sysfs_delete_snapshot_group(root);
> -		iput(root->ifile);
> +		mutex_unlock(&nilfs->ns_cptree_lock);
> 
> +		wait_for_completion(&root-
> >snapshot_kobj_unregister);

Ditto.

Thanks,
Slava.

> +		iput(root->ifile);
>  		kfree(root);
>  	}
>  }
> diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
> index 4776a70f01..074644c64a 100644
> --- a/fs/nilfs2/the_nilfs.h
> +++ b/fs/nilfs2/the_nilfs.h
> @@ -150,7 +150,7 @@ struct the_nilfs {
> 
>  	/* Checkpoint tree */
>  	struct rb_root		ns_cptree;
> -	spinlock_t		ns_cptree_lock;
> +	struct mutex		ns_cptree_lock; /* Protects
> ns_cptree */
> 
>  	/* Dirty inode list */
>  	struct list_head	ns_dirty_files;
> --
> 2.43.0

Hi Slava,

No, wait_for_completion() without a timeout is intentional here.

The kobject subsystem guarantees that the release callback will
eventually run -- CONFIG_DEBUG_KOBJECT_RELEASE only defers it, it
never drops it. So the wait is bounded in practice.

If we used wait_for_completion_killable_timeout() and the timeout
fired (or a signal arrived) before the release callback ran, we
would kfree() the root while the kobject release is still pending.
When the callback finally runs it would access freed memory -- a
use-after-free.

wait_for_completion_killable() (without timeout) has the same
problem: if the wait is interrupted by a fatal signal, we cannot
safely free the container because the release callback may still
reference it.

The unconditional wait_for_completion() is the only safe choice
when the caller must free the container of an embedded kobject.
This is the same pattern used by other subsystems (e.g.,
blk_mq_tag_set, configfs subsystems).

Aldo

On Sat, Sep 19, 2026 at 6:22 AM Aldo Ariel Panzardo wrote:
>
> Hi Slava,
>
> No, wait_for_completion() without a timeout is intentional here.
>
> The kobject subsystem guarantees that the release callback will
> eventually run -- CONFIG_DEBUG_KOBJECT_RELEASE only defers it, it
> never drops it. So the wait is bounded in practice.
>
> If we used wait_for_completion_killable_timeout() and the timeout
> fired (or a signal arrived) before the release callback ran, we
> would kfree() the root while the kobject release is still pending.
> When the callback finally runs it would access freed memory -- a
> use-after-free.
>
> wait_for_completion_killable() (without timeout) has the same
> problem: if the wait is interrupted by a fatal signal, we cannot
> safely free the container because the release callback may still
> reference it.
>
> The unconditional wait_for_completion() is the only safe choice
> when the caller must free the container of an embedded kobject.
> This is the same pattern used by other subsystems (e.g.,
> blk_mq_tag_set, configfs subsystems).
>
> Aldo

Hi Aldo,

Thank you for the patch.

However, in my view, introducing a mutex for synchronization (either
by replacing the spinlock or adding a mutex alongside it) is
unnecessary and suboptimal.

nilfs_sysfs_create_snapshot_group() performs memory allocations and
sysfs node creation, which can sleep.  Holding 'ns_cptree_lock' across
this operation causes concurrent read-only callers like
nilfs_lookup_root() to block unnecessarily on a lock that originally
protected a very fast, in-memory rbtree lookup.

Note that checkpoint and snapshot mounts themselves are serialized by
'nilfs->ns_snapshot_mount_mutex', so strict mutual exclusion between
concurrent insertions is not required; we only need to eliminate the
race against nilfs_lookup_root().

Instead of converting 'ns_cptree_lock' to a mutex, I think it would be
cleaner to keep 'ns_cptree_lock' as a spinlock and structure the
creation flow so that new is fully initialized and registered with
sysfs *before* acquiring the spinlock:

1. Allocate new and initialize the basic fields required for sysfs
creation (cno, nilfs, etc.).
2. Call nilfs_sysfs_create_snapshot_group(new) *before* taking the lock.
    - On error, wait_for_completion(&new->snapshot_kobj_unregister),
      kfree(new), and return NULL.
3. If sysfs creation succeeds, acquire spin_lock(&nilfs->ns_cptree_lock),
   link new into ns_cptree, and drop the spinlock.

In nilfs_put_root(), we can keep refcount_dec_and_lock() with the
spinlock, erase the node from the tree, drop the spinlock, delete the
sysfs group, wait for completion, and free root.

Could you please update the patch to keep 'ns_cptree_lock' as a
spinlock and complete sysfs registration before inserting the root
into ns_cptree?

I think this approach should work, but please let me know if I've
missed anything.

Thanks,
Ryusuke Konishi

On Mon, Sep 21, 2026 at 4:47 PM Ryusuke Konishi wrote:
>
> On Sat, Sep 19, 2026 at 6:22 AM Aldo Ariel Panzardo wrote:
> >
> > Hi Slava,
> >
> > No, wait_for_completion() without a timeout is intentional here.
> >
> > The kobject subsystem guarantees that the release callback will
> > eventually run -- CONFIG_DEBUG_KOBJECT_RELEASE only defers it, it
> > never drops it. So the wait is bounded in practice.
> >
> > If we used wait_for_completion_killable_timeout() and the timeout
> > fired (or a signal arrived) before the release callback ran, we
> > would kfree() the root while the kobject release is still pending.
> > When the callback finally runs it would access freed memory -- a
> > use-after-free.
> >
> > wait_for_completion_killable() (without timeout) has the same
> > problem: if the wait is interrupted by a fatal signal, we cannot
> > safely free the container because the release callback may still
> > reference it.
> >
> > The unconditional wait_for_completion() is the only safe choice
> > when the caller must free the container of an embedded kobject.
> > This is the same pattern used by other subsystems (e.g.,
> > blk_mq_tag_set, configfs subsystems).
> >
> > Aldo
>
> Hi Aldo,
>
> Thank you for the patch.
>
> However, in my view, introducing a mutex for synchronization (either
> by replacing the spinlock or adding a mutex alongside it) is
> unnecessary and suboptimal.
>
> nilfs_sysfs_create_snapshot_group() performs memory allocations and
> sysfs node creation, which can sleep.  Holding 'ns_cptree_lock' across
> this operation causes concurrent read-only callers like
> nilfs_lookup_root() to block unnecessarily on a lock that originally
> protected a very fast, in-memory rbtree lookup.
>
> Note that checkpoint and snapshot mounts themselves are serialized by
> 'nilfs->ns_snapshot_mount_mutex', so strict mutual exclusion between
> concurrent insertions is not required; we only need to eliminate the
> race against nilfs_lookup_root().
>
> Instead of converting 'ns_cptree_lock' to a mutex, I think it would be
> cleaner to keep 'ns_cptree_lock' as a spinlock and structure the
> creation flow so that new is fully initialized and registered with
> sysfs *before* acquiring the spinlock:
>
> 1. Allocate new and initialize the basic fields required for sysfs
> creation (cno, nilfs, etc.).
> 2. Call nilfs_sysfs_create_snapshot_group(new) *before* taking the lock.
>     - On error, wait_for_completion(&new->snapshot_kobj_unregister),
>       kfree(new), and return NULL.
> 3. If sysfs creation succeeds, acquire spin_lock(&nilfs->ns_cptree_lock),
>    link new into ns_cptree, and drop the spinlock.
>
> In nilfs_put_root(), we can keep refcount_dec_and_lock() with the
> spinlock, erase the node from the tree, drop the spinlock, delete the
> sysfs group, wait for completion, and free root.
>
> Could you please update the patch to keep 'ns_cptree_lock' as a
> spinlock and complete sysfs registration before inserting the root
> into ns_cptree?
>
> I think this approach should work, but please let me know if I've
> missed anything.
>
> Thanks,
> Ryusuke Konishi

Hi Aldo,

On second thought, regarding
wait_for_completion(&root->snapshot_kobj_unregister),
wouldn't it be even cleaner to keep it inside the sysfs helper
functions instead of exposing it in the cptree functions?

Specifically:

- Inside nilfs_sysfs_create_snapshot_group():
  Call wait_for_completion(&root->snapshot_kobj_unregister) directly in its
  error path after kobject_put().

- Inside nilfs_sysfs_delete_snapshot_group():
  Call wait_for_completion(&root->snapshot_kobj_unregister) after kobject_put().

This way, the completion logic is nicely encapsulated within the sysfs
subsystem, so nilfs_find_or_create_root() and nilfs_put_root() don't
need to manage the internal completion state explicitly they can
simply call the sysfs functions and perform kfree() when needed.

What do you think?

Thanks,
Ryusuke Konishi

In-Reply-To: <CAKFNMonYVUbzjYm343Aw-=xjUk78x4EdUwGBfNRwE6zfmRVhvA@mail.gmail.com>



Hi Ryusuke,

Completely agree — that is cleaner.  v4 (below) does exactly this:

  nilfs_sysfs_create_snapshot_group():
    error path does kobject_put() + wait_for_completion()
    
  nilfs_sysfs_delete_snapshot_group():
    kobject_put() + wait_for_completion()
    
So nilfs_find_or_create_root() and nilfs_put_root() just call the
helpers and kfree(); they never touch the completion directly.

v4 also keeps ns_cptree_lock as a spinlock and creates the sysfs
group before taking the lock, per your earlier message.

Aldo

nilfs_find_or_create_root() links a new checkpoint root into the
checkpoint tree and drops ns_cptree_lock before creating the root's
sysfs group.  If nilfs_sysfs_create_snapshot_group() then fails, the
root is freed while it is still reachable from ns_cptree, so a
concurrent nilfs_lookup_root() can dereference freed memory.

nilfs_sysfs_create_snapshot_group() allocates memory and creates sysfs
nodes and may sleep, so it cannot run under the ns_cptree_lock spinlock.
Reorder the creation path so that the sysfs group is set up before the
root is published: initialize the root, create its sysfs group outside
the lock, and only then take ns_cptree_lock to link it into the tree.
On sysfs failure the root was never visible and is freed directly; in
the unlikely case a root with the same checkpoint number is already
present, the sysfs group of the new root is removed before it is freed.
Concurrent insertions are already serialized by ns_snapshot_mount_mutex,
so only the race against nilfs_lookup_root() needs to be closed.

Because kobject_put() does not guarantee that the embedded kobject's
release callback has run by the time it returns
(CONFIG_DEBUG_KOBJECT_RELEASE defers it), the container must not be
freed until the release completes.  Keep that wait_for_completion() of
snapshot_kobj_unregister inside the sysfs helpers, so both the creation
error path and nilfs_sysfs_delete_snapshot_group() wait for the release
before their callers free the root.

Fixes: dd70edbde262 ("nilfs2: integrate sysfs support into driver")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
Changes in v4:
  - Keep ns_cptree_lock as a spinlock instead of converting it to a
    mutex, and create the sysfs group before taking the lock, as
    suggested by Ryusuke Konishi. Concurrent insertions are already
    serialized by ns_snapshot_mount_mutex; only the race against
    nilfs_lookup_root() needs to be closed.
  - Move the wait_for_completion() of snapshot_kobj_unregister into the
    sysfs helpers (create error path and delete), so the checkpoint
    tree functions no longer manage the kobject completion state, as
    suggested by Ryusuke Konishi.

Changes in v3:
  - Move wait_for_completion() out of the locked section in
    nilfs_put_root() (the mutex-based v2/v3 approach; dropped in v4).

Changes in v2:
  - Serialize the sysfs registration against the tree instead of adding
    a second lock, as suggested by Viacheslav Dubeyko.

 fs/nilfs2/sysfs.c     |  5 ++++-
 fs/nilfs2/the_nilfs.c | 34 +++++++++++++++++++++-------------
 2 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
index bc52afbfc5c7..5bb74a45ad1e 100644
--- a/fs/nilfs2/sysfs.c
+++ b/fs/nilfs2/sysfs.c
@@ -195,8 +195,10 @@ int nilfs_sysfs_create_snapshot_group(struct nilfs_root *root)
 					    "%llu", root->cno);
 	}
 
-	if (err)
+	if (err) {
 		kobject_put(&root->snapshot_kobj);
+		wait_for_completion(&root->snapshot_kobj_unregister);
+	}
 
 	return err;
 }
@@ -204,6 +206,7 @@ int nilfs_sysfs_create_snapshot_group(struct nilfs_root *root)
 void nilfs_sysfs_delete_snapshot_group(struct nilfs_root *root)
 {
 	kobject_put(&root->snapshot_kobj);
+	wait_for_completion(&root->snapshot_kobj_unregister);
 }
 
 /************************************************************************
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index ecd71c190885..1d5d2293c9c7 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -869,6 +869,26 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
 	if (!new)
 		return NULL;
 
+	new->cno = cno;
+	new->ifile = NULL;
+	new->nilfs = nilfs;
+	refcount_set(&new->count, 1);
+	atomic64_set(&new->inodes_count, 0);
+	atomic64_set(&new->blocks_count, 0);
+
+	/*
+	 * Register the sysfs group before publishing the root in the
+	 * checkpoint tree.  nilfs_sysfs_create_snapshot_group() can sleep,
+	 * so it must run outside ns_cptree_lock; creating it first also
+	 * ensures a concurrent nilfs_lookup_root() can never observe a root
+	 * whose sysfs registration later fails and gets freed.
+	 */
+	err = nilfs_sysfs_create_snapshot_group(new);
+	if (err) {
+		kfree(new);
+		return NULL;
+	}
+
 	spin_lock(&nilfs->ns_cptree_lock);
 
 	p = &nilfs->ns_cptree.rb_node;
@@ -885,29 +905,17 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
 		} else {
 			refcount_inc(&root->count);
 			spin_unlock(&nilfs->ns_cptree_lock);
+			nilfs_sysfs_delete_snapshot_group(new);
 			kfree(new);
 			return root;
 		}
 	}
 
-	new->cno = cno;
-	new->ifile = NULL;
-	new->nilfs = nilfs;
-	refcount_set(&new->count, 1);
-	atomic64_set(&new->inodes_count, 0);
-	atomic64_set(&new->blocks_count, 0);
-
 	rb_link_node(&new->rb_node, parent, p);
 	rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
 
 	spin_unlock(&nilfs->ns_cptree_lock);
 
-	err = nilfs_sysfs_create_snapshot_group(new);
-	if (err) {
-		kfree(new);
-		new = NULL;
-	}
-
 	return new;
 }
 
-- 
2.43.0

(Resending due to a delivery failure with the original recipient address.)

On Tue, Sep 22, 2026 at 12:22 AM Aldo Ariel Panzardo wrote:
>
> nilfs_find_or_create_root() links a new checkpoint root into the
> checkpoint tree and drops ns_cptree_lock before creating the root's
> sysfs group.  If nilfs_sysfs_create_snapshot_group() then fails, the
> root is freed while it is still reachable from ns_cptree, so a
> concurrent nilfs_lookup_root() can dereference freed memory.
>
> nilfs_sysfs_create_snapshot_group() allocates memory and creates sysfs
> nodes and may sleep, so it cannot run under the ns_cptree_lock spinlock.
> Reorder the creation path so that the sysfs group is set up before the
> root is published: initialize the root, create its sysfs group outside
> the lock, and only then take ns_cptree_lock to link it into the tree.
> On sysfs failure the root was never visible and is freed directly; in
> the unlikely case a root with the same checkpoint number is already
> present, the sysfs group of the new root is removed before it is freed.
> Concurrent insertions are already serialized by ns_snapshot_mount_mutex,
> so only the race against nilfs_lookup_root() needs to be closed.
>
> Because kobject_put() does not guarantee that the embedded kobject's
> release callback has run by the time it returns
> (CONFIG_DEBUG_KOBJECT_RELEASE defers it), the container must not be
> freed until the release completes.  Keep that wait_for_completion() of
> snapshot_kobj_unregister inside the sysfs helpers, so both the creation
> error path and nilfs_sysfs_delete_snapshot_group() wait for the release
> before their callers free the root.
>
> Fixes: dd70edbde262 ("nilfs2: integrate sysfs support into driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
> Changes in v4:
>   - Keep ns_cptree_lock as a spinlock instead of converting it to a
>     mutex, and create the sysfs group before taking the lock, as
>     suggested by Ryusuke Konishi. Concurrent insertions are already
>     serialized by ns_snapshot_mount_mutex; only the race against
>     nilfs_lookup_root() needs to be closed.
>   - Move the wait_for_completion() of snapshot_kobj_unregister into the
>     sysfs helpers (create error path and delete), so the checkpoint
>     tree functions no longer manage the kobject completion state, as
>     suggested by Ryusuke Konishi.
>
> Changes in v3:
>   - Move wait_for_completion() out of the locked section in
>     nilfs_put_root() (the mutex-based v2/v3 approach; dropped in v4).
>
> Changes in v2:
>   - Serialize the sysfs registration against the tree instead of adding
>     a second lock, as suggested by Viacheslav Dubeyko.

Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>

Looks good to me.

Viacheslav, could you please apply this v4 patch directly?
(Please keep the 'Cc: stable' tag this time, since this fixes a
Use-After-Free bug.)

One note regarding this change: if a sysfs directory corresponding to
the checkpoint already exists, nilfs_sysfs_create_snapshot_group()
will not only return an error, but kobject_init_and_add() will also
output an
error message.

However, nilfs_find_or_create_root() checks for an existing entry at
the beginning using nilfs_lookup_root(), and
nilfs_attach_checkpoint(), which calls this function, is always
invoked exclusively — either under 'ns_snapshot_mount_mutex' or during
the serialized initial mount path.
Therefore, this side effect is avoided.

Thanks,
Ryusuke Konishi

>
>  fs/nilfs2/sysfs.c     |  5 ++++-
>  fs/nilfs2/the_nilfs.c | 34 +++++++++++++++++++++-------------
>  2 files changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
> index bc52afbfc5c7..5bb74a45ad1e 100644
> --- a/fs/nilfs2/sysfs.c
> +++ b/fs/nilfs2/sysfs.c
> @@ -195,8 +195,10 @@ int nilfs_sysfs_create_snapshot_group(struct nilfs_root *root)
>                                             "%llu", root->cno);
>         }
>
> -       if (err)
> +       if (err) {
>                 kobject_put(&root->snapshot_kobj);
> +               wait_for_completion(&root->snapshot_kobj_unregister);
> +       }
>
>         return err;
>  }
> @@ -204,6 +206,7 @@ int nilfs_sysfs_create_snapshot_group(struct nilfs_root *root)
>  void nilfs_sysfs_delete_snapshot_group(struct nilfs_root *root)
>  {
>         kobject_put(&root->snapshot_kobj);
> +       wait_for_completion(&root->snapshot_kobj_unregister);
>  }
>
>  /************************************************************************
> diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> index ecd71c190885..1d5d2293c9c7 100644
> --- a/fs/nilfs2/the_nilfs.c
> +++ b/fs/nilfs2/the_nilfs.c
> @@ -869,6 +869,26 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
>         if (!new)
>                 return NULL;
>
> +       new->cno = cno;
> +       new->ifile = NULL;
> +       new->nilfs = nilfs;
> +       refcount_set(&new->count, 1);
> +       atomic64_set(&new->inodes_count, 0);
> +       atomic64_set(&new->blocks_count, 0);
> +
> +       /*
> +        * Register the sysfs group before publishing the root in the
> +        * checkpoint tree.  nilfs_sysfs_create_snapshot_group() can sleep,
> +        * so it must run outside ns_cptree_lock; creating it first also
> +        * ensures a concurrent nilfs_lookup_root() can never observe a root
> +        * whose sysfs registration later fails and gets freed.
> +        */
> +       err = nilfs_sysfs_create_snapshot_group(new);
> +       if (err) {
> +               kfree(new);
> +               return NULL;
> +       }
> +
>         spin_lock(&nilfs->ns_cptree_lock);
>
>         p = &nilfs->ns_cptree.rb_node;
> @@ -885,29 +905,17 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
>                 } else {
>                         refcount_inc(&root->count);
>                         spin_unlock(&nilfs->ns_cptree_lock);
> +                       nilfs_sysfs_delete_snapshot_group(new);
>                         kfree(new);
>                         return root;
>                 }
>         }
>
> -       new->cno = cno;
> -       new->ifile = NULL;
> -       new->nilfs = nilfs;
> -       refcount_set(&new->count, 1);
> -       atomic64_set(&new->inodes_count, 0);
> -       atomic64_set(&new->blocks_count, 0);
> -
>         rb_link_node(&new->rb_node, parent, p);
>         rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
>
>         spin_unlock(&nilfs->ns_cptree_lock);
>
> -       err = nilfs_sysfs_create_snapshot_group(new);
> -       if (err) {
> -               kfree(new);
> -               new = NULL;
> -       }
> -
>         return new;
>  }
>
> --
> 2.43.0
>

On Tue, Sep 22, 2026 at 12:22 AM Aldo Ariel Panzardo wrote:
>
> nilfs_find_or_create_root() links a new checkpoint root into the
> checkpoint tree and drops ns_cptree_lock before creating the root's
> sysfs group.  If nilfs_sysfs_create_snapshot_group() then fails, the
> root is freed while it is still reachable from ns_cptree, so a
> concurrent nilfs_lookup_root() can dereference freed memory.
>
> nilfs_sysfs_create_snapshot_group() allocates memory and creates sysfs
> nodes and may sleep, so it cannot run under the ns_cptree_lock spinlock.
> Reorder the creation path so that the sysfs group is set up before the
> root is published: initialize the root, create its sysfs group outside
> the lock, and only then take ns_cptree_lock to link it into the tree.
> On sysfs failure the root was never visible and is freed directly; in
> the unlikely case a root with the same checkpoint number is already
> present, the sysfs group of the new root is removed before it is freed.
> Concurrent insertions are already serialized by ns_snapshot_mount_mutex,
> so only the race against nilfs_lookup_root() needs to be closed.
>
> Because kobject_put() does not guarantee that the embedded kobject's
> release callback has run by the time it returns
> (CONFIG_DEBUG_KOBJECT_RELEASE defers it), the container must not be
> freed until the release completes.  Keep that wait_for_completion() of
> snapshot_kobj_unregister inside the sysfs helpers, so both the creation
> error path and nilfs_sysfs_delete_snapshot_group() wait for the release
> before their callers free the root.
>
> Fixes: dd70edbde262 ("nilfs2: integrate sysfs support into driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
> Changes in v4:
>   - Keep ns_cptree_lock as a spinlock instead of converting it to a
>     mutex, and create the sysfs group before taking the lock, as
>     suggested by Ryusuke Konishi. Concurrent insertions are already
>     serialized by ns_snapshot_mount_mutex; only the race against
>     nilfs_lookup_root() needs to be closed.
>   - Move the wait_for_completion() of snapshot_kobj_unregister into the
>     sysfs helpers (create error path and delete), so the checkpoint
>     tree functions no longer manage the kobject completion state, as
>     suggested by Ryusuke Konishi.
>
> Changes in v3:
>   - Move wait_for_completion() out of the locked section in
>     nilfs_put_root() (the mutex-based v2/v3 approach; dropped in v4).
>
> Changes in v2:
>   - Serialize the sysfs registration against the tree instead of adding
>     a second lock, as suggested by Viacheslav Dubeyko.

Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>

Looks good to me.

Viacheslav, could you please apply this v4 patch directly?
(Please keep the 'Cc: stable' tag this time, since this fixes a
Use-After-Free bug.)

One note regarding this change: if a sysfs directory corresponding to
the checkpoint already exists, nilfs_sysfs_create_snapshot_group()
will not only return an error, but kobject_init_and_add() will also
output an
error message.

However, nilfs_find_or_create_root() checks for an existing entry at
the beginning using nilfs_lookup_root(), and
nilfs_attach_checkpoint(), which calls this function, is always
invoked exclusively — either under 'ns_snapshot_mount_mutex' or during
the serialized initial mount path.
Therefore, this side effect is avoided.

Thanks,
Ryusuke Konishi

>
>  fs/nilfs2/sysfs.c     |  5 ++++-
>  fs/nilfs2/the_nilfs.c | 34 +++++++++++++++++++++-------------
>  2 files changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
> index bc52afbfc5c7..5bb74a45ad1e 100644
> --- a/fs/nilfs2/sysfs.c
> +++ b/fs/nilfs2/sysfs.c
> @@ -195,8 +195,10 @@ int nilfs_sysfs_create_snapshot_group(struct nilfs_root *root)
>                                             "%llu", root->cno);
>         }
>
> -       if (err)
> +       if (err) {
>                 kobject_put(&root->snapshot_kobj);
> +               wait_for_completion(&root->snapshot_kobj_unregister);
> +       }
>
>         return err;
>  }
> @@ -204,6 +206,7 @@ int nilfs_sysfs_create_snapshot_group(struct nilfs_root *root)
>  void nilfs_sysfs_delete_snapshot_group(struct nilfs_root *root)
>  {
>         kobject_put(&root->snapshot_kobj);
> +       wait_for_completion(&root->snapshot_kobj_unregister);
>  }
>
>  /************************************************************************
> diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> index ecd71c190885..1d5d2293c9c7 100644
> --- a/fs/nilfs2/the_nilfs.c
> +++ b/fs/nilfs2/the_nilfs.c
> @@ -869,6 +869,26 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
>         if (!new)
>                 return NULL;
>
> +       new->cno = cno;
> +       new->ifile = NULL;
> +       new->nilfs = nilfs;
> +       refcount_set(&new->count, 1);
> +       atomic64_set(&new->inodes_count, 0);
> +       atomic64_set(&new->blocks_count, 0);
> +
> +       /*
> +        * Register the sysfs group before publishing the root in the
> +        * checkpoint tree.  nilfs_sysfs_create_snapshot_group() can sleep,
> +        * so it must run outside ns_cptree_lock; creating it first also
> +        * ensures a concurrent nilfs_lookup_root() can never observe a root
> +        * whose sysfs registration later fails and gets freed.
> +        */
> +       err = nilfs_sysfs_create_snapshot_group(new);
> +       if (err) {
> +               kfree(new);
> +               return NULL;
> +       }
> +
>         spin_lock(&nilfs->ns_cptree_lock);
>
>         p = &nilfs->ns_cptree.rb_node;
> @@ -885,29 +905,17 @@ nilfs_find_or_create_root(struct the_nilfs *nilfs, __u64 cno)
>                 } else {
>                         refcount_inc(&root->count);
>                         spin_unlock(&nilfs->ns_cptree_lock);
> +                       nilfs_sysfs_delete_snapshot_group(new);
>                         kfree(new);
>                         return root;
>                 }
>         }
>
> -       new->cno = cno;
> -       new->ifile = NULL;
> -       new->nilfs = nilfs;
> -       refcount_set(&new->count, 1);
> -       atomic64_set(&new->inodes_count, 0);
> -       atomic64_set(&new->blocks_count, 0);
> -
>         rb_link_node(&new->rb_node, parent, p);
>         rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
>
>         spin_unlock(&nilfs->ns_cptree_lock);
>
> -       err = nilfs_sysfs_create_snapshot_group(new);
> -       if (err) {
> -               kfree(new);
> -               new = NULL;
> -       }
> -
>         return new;
>  }
>
> --
> 2.43.0
>

On Tue, 2026-09-22 at 08:55 +0900, Ryusuke Konishi wrote:
> On Tue, Sep 22, 2026 at 12:22 AM Aldo Ariel Panzardo wrote:
> > 
> > nilfs_find_or_create_root() links a new checkpoint root into the
> > checkpoint tree and drops ns_cptree_lock before creating the root's
> > sysfs group.  If nilfs_sysfs_create_snapshot_group() then fails,
> > the
> > root is freed while it is still reachable from ns_cptree, so a
> > concurrent nilfs_lookup_root() can dereference freed memory.
> > 
> > nilfs_sysfs_create_snapshot_group() allocates memory and creates
> > sysfs
> > nodes and may sleep, so it cannot run under the ns_cptree_lock
> > spinlock.
> > Reorder the creation path so that the sysfs group is set up before
> > the
> > root is published: initialize the root, create its sysfs group
> > outside
> > the lock, and only then take ns_cptree_lock to link it into the
> > tree.
> > On sysfs failure the root was never visible and is freed directly;
> > in
> > the unlikely case a root with the same checkpoint number is already
> > present, the sysfs group of the new root is removed before it is
> > freed.
> > Concurrent insertions are already serialized by
> > ns_snapshot_mount_mutex,
> > so only the race against nilfs_lookup_root() needs to be closed.
> > 
> > Because kobject_put() does not guarantee that the embedded
> > kobject's
> > release callback has run by the time it returns
> > (CONFIG_DEBUG_KOBJECT_RELEASE defers it), the container must not be
> > freed until the release completes.  Keep that wait_for_completion()
> > of
> > snapshot_kobj_unregister inside the sysfs helpers, so both the
> > creation
> > error path and nilfs_sysfs_delete_snapshot_group() wait for the
> > release
> > before their callers free the root.
> > 
> > Fixes: dd70edbde262 ("nilfs2: integrate sysfs support into driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> > ---
> > Changes in v4:
> >   - Keep ns_cptree_lock as a spinlock instead of converting it to a
> >     mutex, and create the sysfs group before taking the lock, as
> >     suggested by Ryusuke Konishi. Concurrent insertions are already
> >     serialized by ns_snapshot_mount_mutex; only the race against
> >     nilfs_lookup_root() needs to be closed.
> >   - Move the wait_for_completion() of snapshot_kobj_unregister into
> > the
> >     sysfs helpers (create error path and delete), so the checkpoint
> >     tree functions no longer manage the kobject completion state,
> > as
> >     suggested by Ryusuke Konishi.
> > 
> > Changes in v3:
> >   - Move wait_for_completion() out of the locked section in
> >     nilfs_put_root() (the mutex-based v2/v3 approach; dropped in
> > v4).
> > 
> > Changes in v2:
> >   - Serialize the sysfs registration against the tree instead of
> > adding
> >     a second lock, as suggested by Viacheslav Dubeyko.
> 
> Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> 
> Looks good to me.
> 
> Viacheslav, could you please apply this v4 patch directly?
> (Please keep the 'Cc: stable' tag this time, since this fixes a
> Use-After-Free bug.)
> 
> One note regarding this change: if a sysfs directory corresponding to
> the checkpoint already exists, nilfs_sysfs_create_snapshot_group()
> will not only return an error, but kobject_init_and_add() will also
> output an
> error message.
> 
> However, nilfs_find_or_create_root() checks for an existing entry at
> the beginning using nilfs_lookup_root(), and
> nilfs_attach_checkpoint(), which calls this function, is always
> invoked exclusively — either under 'ns_snapshot_mount_mutex' or
> during
> the serialized initial mount path.
> Therefore, this side effect is avoided.
> 
> Thanks,
> Ryusuke Konishi
> 
> > 
> >  fs/nilfs2/sysfs.c     |  5 ++++-
> >  fs/nilfs2/the_nilfs.c | 34 +++++++++++++++++++++-------------
> >  2 files changed, 25 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/nilfs2/sysfs.c b/fs/nilfs2/sysfs.c
> > index bc52afbfc5c7..5bb74a45ad1e 100644
> > --- a/fs/nilfs2/sysfs.c
> > +++ b/fs/nilfs2/sysfs.c
> > @@ -195,8 +195,10 @@ int nilfs_sysfs_create_snapshot_group(struct
> > nilfs_root *root)
> >                                             "%llu", root->cno);
> >         }
> > 
> > -       if (err)
> > +       if (err) {
> >                 kobject_put(&root->snapshot_kobj);
> > +               wait_for_completion(&root-
> > >snapshot_kobj_unregister);
> > +       }
> > 
> >         return err;
> >  }
> > @@ -204,6 +206,7 @@ int nilfs_sysfs_create_snapshot_group(struct
> > nilfs_root *root)
> >  void nilfs_sysfs_delete_snapshot_group(struct nilfs_root *root)
> >  {
> >         kobject_put(&root->snapshot_kobj);
> > +       wait_for_completion(&root->snapshot_kobj_unregister);
> >  }
> > 
> >  /*****************************************************************
> > *******
> > diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
> > index ecd71c190885..1d5d2293c9c7 100644
> > --- a/fs/nilfs2/the_nilfs.c
> > +++ b/fs/nilfs2/the_nilfs.c
> > @@ -869,6 +869,26 @@ nilfs_find_or_create_root(struct the_nilfs
> > *nilfs, __u64 cno)
> >         if (!new)
> >                 return NULL;
> > 
> > +       new->cno = cno;
> > +       new->ifile = NULL;
> > +       new->nilfs = nilfs;
> > +       refcount_set(&new->count, 1);
> > +       atomic64_set(&new->inodes_count, 0);
> > +       atomic64_set(&new->blocks_count, 0);
> > +
> > +       /*
> > +        * Register the sysfs group before publishing the root in
> > the
> > +        * checkpoint tree.  nilfs_sysfs_create_snapshot_group()
> > can sleep,
> > +        * so it must run outside ns_cptree_lock; creating it first
> > also
> > +        * ensures a concurrent nilfs_lookup_root() can never
> > observe a root
> > +        * whose sysfs registration later fails and gets freed.
> > +        */
> > +       err = nilfs_sysfs_create_snapshot_group(new);
> > +       if (err) {
> > +               kfree(new);
> > +               return NULL;
> > +       }
> > +
> >         spin_lock(&nilfs->ns_cptree_lock);
> > 
> >         p = &nilfs->ns_cptree.rb_node;
> > @@ -885,29 +905,17 @@ nilfs_find_or_create_root(struct the_nilfs
> > *nilfs, __u64 cno)
> >                 } else {
> >                         refcount_inc(&root->count);
> >                         spin_unlock(&nilfs->ns_cptree_lock);
> > +                       nilfs_sysfs_delete_snapshot_group(new);
> >                         kfree(new);
> >                         return root;
> >                 }
> >         }
> > 
> > -       new->cno = cno;
> > -       new->ifile = NULL;
> > -       new->nilfs = nilfs;
> > -       refcount_set(&new->count, 1);
> > -       atomic64_set(&new->inodes_count, 0);
> > -       atomic64_set(&new->blocks_count, 0);
> > -
> >         rb_link_node(&new->rb_node, parent, p);
> >         rb_insert_color(&new->rb_node, &nilfs->ns_cptree);
> > 
> >         spin_unlock(&nilfs->ns_cptree_lock);
> > 
> > -       err = nilfs_sysfs_create_snapshot_group(new);
> > -       if (err) {
> > -               kfree(new);
> > -               new = NULL;
> > -       }
> > -
> >         return new;
> >  }
> > 
> > --
> > 2.43.0
> > 

Applied.

Thanks,
Slava.