public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] btrfs: fix use-after-free warning in btrfs_get_or_create_delayed_node
@ 2025-12-02 21:21 Leo Martins
  2025-12-03 10:50 ` Filipe Manana
  0 siblings, 1 reply; 5+ messages in thread
From: Leo Martins @ 2025-12-02 21:21 UTC (permalink / raw)
  To: linux-btrfs, kernel-team; +Cc: kernel test robot

Previously, btrfs_get_or_create_delayed_node sets the delayed_node's
refcount before acquiring the root->delayed_nodes lock.
Commit e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
moves refcount_set inside the critical section which means
there is no longer a memory barrier between setting the refcount and
setting btrfs_inode->delayed_node = node.

This allows btrfs_get_or_create_delayed_node to set
btrfs_inode->delayed_node before setting the refcount.
A different thread is then able to read and increase the refcount
of btrfs_inode->delayed_node leading to a refcounting bug and
a use-after-free warning.

The fix is to move refcount_set back to where it was to take
advantage of the implicit memory barrier provided by lock
acquisition.

Fixes: e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202511262228.6dda231e-lkp@intel.com
Tested-by: kernel test robot <oliver.sang@intel.com>
Signed-off-by: Leo Martins <loemra.dev@gmail.com>
---
 fs/btrfs/delayed-inode.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index 364814642a91..81922556379d 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -152,37 +152,39 @@ static struct btrfs_delayed_node *btrfs_get_or_create_delayed_node(
 		return ERR_PTR(-ENOMEM);
 	btrfs_init_delayed_node(node, root, ino);
 
+	/* Cached in the inode and can be accessed. */
+	refcount_set(&node->refs, 2);
+	btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
+	btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
+
 	/* Allocate and reserve the slot, from now it can return a NULL from xa_load(). */
 	ret = xa_reserve(&root->delayed_nodes, ino, GFP_NOFS);
-	if (ret == -ENOMEM) {
-		btrfs_delayed_node_ref_tracker_dir_exit(node);
-		kmem_cache_free(delayed_node_cache, node);
-		return ERR_PTR(-ENOMEM);
-	}
+	if (ret == -ENOMEM)
+		goto cleanup;
+
 	xa_lock(&root->delayed_nodes);
 	ptr = xa_load(&root->delayed_nodes, ino);
 	if (ptr) {
 		/* Somebody inserted it, go back and read it. */
 		xa_unlock(&root->delayed_nodes);
-		btrfs_delayed_node_ref_tracker_dir_exit(node);
-		kmem_cache_free(delayed_node_cache, node);
-		node = NULL;
-		goto again;
+		goto cleanup;
 	}
 	ptr = __xa_store(&root->delayed_nodes, ino, node, GFP_ATOMIC);
 	ASSERT(xa_err(ptr) != -EINVAL);
 	ASSERT(xa_err(ptr) != -ENOMEM);
 	ASSERT(ptr == NULL);
-
-	/* Cached in the inode and can be accessed. */
-	refcount_set(&node->refs, 2);
-	btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
-	btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
-
 	btrfs_inode->delayed_node = node;
 	xa_unlock(&root->delayed_nodes);
 
 	return node;
+cleanup:
+	btrfs_delayed_node_ref_tracker_free(node, tracker);
+	btrfs_delayed_node_ref_tracker_free(node, &node->inode_cache_tracker);
+	btrfs_delayed_node_ref_tracker_dir_exit(node);
+	kmem_cache_free(delayed_node_cache, node);
+	if (ret)
+		return ERR_PTR(ret);
+	goto again;
 }
 
 /*
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] btrfs: fix use-after-free warning in btrfs_get_or_create_delayed_node
  2025-12-02 21:21 [PATCH v2] btrfs: fix use-after-free warning in btrfs_get_or_create_delayed_node Leo Martins
@ 2025-12-03 10:50 ` Filipe Manana
  2025-12-12  1:01   ` Leo Martins
  0 siblings, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2025-12-03 10:50 UTC (permalink / raw)
  To: Leo Martins; +Cc: linux-btrfs, kernel-team, kernel test robot

On Tue, Dec 2, 2025 at 9:21 PM Leo Martins <loemra.dev@gmail.com> wrote:
>
> Previously, btrfs_get_or_create_delayed_node sets the delayed_node's
> refcount before acquiring the root->delayed_nodes lock.
> Commit e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
> moves refcount_set inside the critical section which means
> there is no longer a memory barrier between setting the refcount and
> setting btrfs_inode->delayed_node = node.
>
> This allows btrfs_get_or_create_delayed_node to set
> btrfs_inode->delayed_node before setting the refcount.
> A different thread is then able to read and increase the refcount
> of btrfs_inode->delayed_node leading to a refcounting bug and
> a use-after-free warning.
>
> The fix is to move refcount_set back to where it was to take
> advantage of the implicit memory barrier provided by lock
> acquisition.
>
> Fixes: e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202511262228.6dda231e-lkp@intel.com
> Tested-by: kernel test robot <oliver.sang@intel.com>
> Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> ---
>  fs/btrfs/delayed-inode.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index 364814642a91..81922556379d 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -152,37 +152,39 @@ static struct btrfs_delayed_node *btrfs_get_or_create_delayed_node(
>                 return ERR_PTR(-ENOMEM);
>         btrfs_init_delayed_node(node, root, ino);
>
> +       /* Cached in the inode and can be accessed. */
> +       refcount_set(&node->refs, 2);
> +       btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
> +       btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);

Now that we do these allocations outside the spinlock (xarray's lock),
we can stop using GFP_ATOMIC and use GFP_NOFS.

Otherwise it looks good, thanks.

> +
>         /* Allocate and reserve the slot, from now it can return a NULL from xa_load(). */
>         ret = xa_reserve(&root->delayed_nodes, ino, GFP_NOFS);
> -       if (ret == -ENOMEM) {
> -               btrfs_delayed_node_ref_tracker_dir_exit(node);
> -               kmem_cache_free(delayed_node_cache, node);
> -               return ERR_PTR(-ENOMEM);
> -       }
> +       if (ret == -ENOMEM)
> +               goto cleanup;
> +
>         xa_lock(&root->delayed_nodes);
>         ptr = xa_load(&root->delayed_nodes, ino);
>         if (ptr) {
>                 /* Somebody inserted it, go back and read it. */
>                 xa_unlock(&root->delayed_nodes);
> -               btrfs_delayed_node_ref_tracker_dir_exit(node);
> -               kmem_cache_free(delayed_node_cache, node);
> -               node = NULL;
> -               goto again;
> +               goto cleanup;
>         }
>         ptr = __xa_store(&root->delayed_nodes, ino, node, GFP_ATOMIC);
>         ASSERT(xa_err(ptr) != -EINVAL);
>         ASSERT(xa_err(ptr) != -ENOMEM);
>         ASSERT(ptr == NULL);
> -
> -       /* Cached in the inode and can be accessed. */
> -       refcount_set(&node->refs, 2);
> -       btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
> -       btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
> -
>         btrfs_inode->delayed_node = node;
>         xa_unlock(&root->delayed_nodes);
>
>         return node;
> +cleanup:
> +       btrfs_delayed_node_ref_tracker_free(node, tracker);
> +       btrfs_delayed_node_ref_tracker_free(node, &node->inode_cache_tracker);
> +       btrfs_delayed_node_ref_tracker_dir_exit(node);
> +       kmem_cache_free(delayed_node_cache, node);
> +       if (ret)
> +               return ERR_PTR(ret);
> +       goto again;
>  }
>
>  /*
> --
> 2.47.3
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] btrfs: fix use-after-free warning in btrfs_get_or_create_delayed_node
  2025-12-03 10:50 ` Filipe Manana
@ 2025-12-12  1:01   ` Leo Martins
  2025-12-12  9:32     ` Filipe Manana
  0 siblings, 1 reply; 5+ messages in thread
From: Leo Martins @ 2025-12-12  1:01 UTC (permalink / raw)
  To: Filipe Manana; +Cc: linux-btrfs, kernel-team, kernel test robot

On Wed, 3 Dec 2025 10:50:17 +0000 Filipe Manana <fdmanana@kernel.org> wrote:

> On Tue, Dec 2, 2025 at 9:21 PM Leo Martins <loemra.dev@gmail.com> wrote:
> >
> > Previously, btrfs_get_or_create_delayed_node sets the delayed_node's
> > refcount before acquiring the root->delayed_nodes lock.
> > Commit e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
> > moves refcount_set inside the critical section which means
> > there is no longer a memory barrier between setting the refcount and
> > setting btrfs_inode->delayed_node = node.
> >
> > This allows btrfs_get_or_create_delayed_node to set
> > btrfs_inode->delayed_node before setting the refcount.
> > A different thread is then able to read and increase the refcount
> > of btrfs_inode->delayed_node leading to a refcounting bug and
> > a use-after-free warning.
> >
> > The fix is to move refcount_set back to where it was to take
> > advantage of the implicit memory barrier provided by lock
> > acquisition.
> >
> > Fixes: e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Closes: https://lore.kernel.org/oe-lkp/202511262228.6dda231e-lkp@intel.com
> > Tested-by: kernel test robot <oliver.sang@intel.com>
> > Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> > ---
> >  fs/btrfs/delayed-inode.c | 32 +++++++++++++++++---------------
> >  1 file changed, 17 insertions(+), 15 deletions(-)
> >
> > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> > index 364814642a91..81922556379d 100644
> > --- a/fs/btrfs/delayed-inode.c
> > +++ b/fs/btrfs/delayed-inode.c
> > @@ -152,37 +152,39 @@ static struct btrfs_delayed_node *btrfs_get_or_create_delayed_node(
> >                 return ERR_PTR(-ENOMEM);
> >         btrfs_init_delayed_node(node, root, ino);
> >
> > +       /* Cached in the inode and can be accessed. */
> > +       refcount_set(&node->refs, 2);
> > +       btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
> > +       btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
> 
> Now that we do these allocations outside the spinlock (xarray's lock),
> we can stop using GFP_ATOMIC and use GFP_NOFS.
> 
> Otherwise it looks good, thanks.

Did you want me to send out a v3 for this change?

> 
> > +
> >         /* Allocate and reserve the slot, from now it can return a NULL from xa_load(). */
> >         ret = xa_reserve(&root->delayed_nodes, ino, GFP_NOFS);
> > -       if (ret == -ENOMEM) {
> > -               btrfs_delayed_node_ref_tracker_dir_exit(node);
> > -               kmem_cache_free(delayed_node_cache, node);
> > -               return ERR_PTR(-ENOMEM);
> > -       }
> > +       if (ret == -ENOMEM)
> > +               goto cleanup;
> > +
> >         xa_lock(&root->delayed_nodes);
> >         ptr = xa_load(&root->delayed_nodes, ino);
> >         if (ptr) {
> >                 /* Somebody inserted it, go back and read it. */
> >                 xa_unlock(&root->delayed_nodes);
> > -               btrfs_delayed_node_ref_tracker_dir_exit(node);
> > -               kmem_cache_free(delayed_node_cache, node);
> > -               node = NULL;
> > -               goto again;
> > +               goto cleanup;
> >         }
> >         ptr = __xa_store(&root->delayed_nodes, ino, node, GFP_ATOMIC);
> >         ASSERT(xa_err(ptr) != -EINVAL);
> >         ASSERT(xa_err(ptr) != -ENOMEM);
> >         ASSERT(ptr == NULL);
> > -
> > -       /* Cached in the inode and can be accessed. */
> > -       refcount_set(&node->refs, 2);
> > -       btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
> > -       btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
> > -
> >         btrfs_inode->delayed_node = node;
> >         xa_unlock(&root->delayed_nodes);
> >
> >         return node;
> > +cleanup:
> > +       btrfs_delayed_node_ref_tracker_free(node, tracker);
> > +       btrfs_delayed_node_ref_tracker_free(node, &node->inode_cache_tracker);
> > +       btrfs_delayed_node_ref_tracker_dir_exit(node);
> > +       kmem_cache_free(delayed_node_cache, node);
> > +       if (ret)
> > +               return ERR_PTR(ret);
> > +       goto again;
> >  }
> >
> >  /*
> > --
> > 2.47.3
> >
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] btrfs: fix use-after-free warning in btrfs_get_or_create_delayed_node
  2025-12-12  1:01   ` Leo Martins
@ 2025-12-12  9:32     ` Filipe Manana
  2025-12-13  1:28       ` Leo Martins
  0 siblings, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2025-12-12  9:32 UTC (permalink / raw)
  To: Leo Martins; +Cc: linux-btrfs, kernel-team, kernel test robot

On Fri, Dec 12, 2025 at 1:01 AM Leo Martins <loemra.dev@gmail.com> wrote:
>
> On Wed, 3 Dec 2025 10:50:17 +0000 Filipe Manana <fdmanana@kernel.org> wrote:
>
> > On Tue, Dec 2, 2025 at 9:21 PM Leo Martins <loemra.dev@gmail.com> wrote:
> > >
> > > Previously, btrfs_get_or_create_delayed_node sets the delayed_node's
> > > refcount before acquiring the root->delayed_nodes lock.
> > > Commit e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
> > > moves refcount_set inside the critical section which means
> > > there is no longer a memory barrier between setting the refcount and
> > > setting btrfs_inode->delayed_node = node.
> > >
> > > This allows btrfs_get_or_create_delayed_node to set
> > > btrfs_inode->delayed_node before setting the refcount.
> > > A different thread is then able to read and increase the refcount
> > > of btrfs_inode->delayed_node leading to a refcounting bug and
> > > a use-after-free warning.
> > >
> > > The fix is to move refcount_set back to where it was to take
> > > advantage of the implicit memory barrier provided by lock
> > > acquisition.
> > >
> > > Fixes: e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
> > > Reported-by: kernel test robot <oliver.sang@intel.com>
> > > Closes: https://lore.kernel.org/oe-lkp/202511262228.6dda231e-lkp@intel.com
> > > Tested-by: kernel test robot <oliver.sang@intel.com>
> > > Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> > > ---
> > >  fs/btrfs/delayed-inode.c | 32 +++++++++++++++++---------------
> > >  1 file changed, 17 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> > > index 364814642a91..81922556379d 100644
> > > --- a/fs/btrfs/delayed-inode.c
> > > +++ b/fs/btrfs/delayed-inode.c
> > > @@ -152,37 +152,39 @@ static struct btrfs_delayed_node *btrfs_get_or_create_delayed_node(
> > >                 return ERR_PTR(-ENOMEM);
> > >         btrfs_init_delayed_node(node, root, ino);
> > >
> > > +       /* Cached in the inode and can be accessed. */
> > > +       refcount_set(&node->refs, 2);
> > > +       btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
> > > +       btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
> >
> > Now that we do these allocations outside the spinlock (xarray's lock),
> > we can stop using GFP_ATOMIC and use GFP_NOFS.
> >
> > Otherwise it looks good, thanks.
>
> Did you want me to send out a v3 for this change?

You can make it either in a v3 or as a separate patch, I don't have a
strong preference for this.

Either way:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

(Applies both to this, a possible v3 and a standalone patch for
replacing GFP_ATOMIC with GFP_NOFS).

If you can't push to for-next, let me know and I can do it.

Thanks.

>
> >
> > > +
> > >         /* Allocate and reserve the slot, from now it can return a NULL from xa_load(). */
> > >         ret = xa_reserve(&root->delayed_nodes, ino, GFP_NOFS);
> > > -       if (ret == -ENOMEM) {
> > > -               btrfs_delayed_node_ref_tracker_dir_exit(node);
> > > -               kmem_cache_free(delayed_node_cache, node);
> > > -               return ERR_PTR(-ENOMEM);
> > > -       }
> > > +       if (ret == -ENOMEM)
> > > +               goto cleanup;
> > > +
> > >         xa_lock(&root->delayed_nodes);
> > >         ptr = xa_load(&root->delayed_nodes, ino);
> > >         if (ptr) {
> > >                 /* Somebody inserted it, go back and read it. */
> > >                 xa_unlock(&root->delayed_nodes);
> > > -               btrfs_delayed_node_ref_tracker_dir_exit(node);
> > > -               kmem_cache_free(delayed_node_cache, node);
> > > -               node = NULL;
> > > -               goto again;
> > > +               goto cleanup;
> > >         }
> > >         ptr = __xa_store(&root->delayed_nodes, ino, node, GFP_ATOMIC);
> > >         ASSERT(xa_err(ptr) != -EINVAL);
> > >         ASSERT(xa_err(ptr) != -ENOMEM);
> > >         ASSERT(ptr == NULL);
> > > -
> > > -       /* Cached in the inode and can be accessed. */
> > > -       refcount_set(&node->refs, 2);
> > > -       btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
> > > -       btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
> > > -
> > >         btrfs_inode->delayed_node = node;
> > >         xa_unlock(&root->delayed_nodes);
> > >
> > >         return node;
> > > +cleanup:
> > > +       btrfs_delayed_node_ref_tracker_free(node, tracker);
> > > +       btrfs_delayed_node_ref_tracker_free(node, &node->inode_cache_tracker);
> > > +       btrfs_delayed_node_ref_tracker_dir_exit(node);
> > > +       kmem_cache_free(delayed_node_cache, node);
> > > +       if (ret)
> > > +               return ERR_PTR(ret);
> > > +       goto again;
> > >  }
> > >
> > >  /*
> > > --
> > > 2.47.3
> > >
> > >

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] btrfs: fix use-after-free warning in btrfs_get_or_create_delayed_node
  2025-12-12  9:32     ` Filipe Manana
@ 2025-12-13  1:28       ` Leo Martins
  0 siblings, 0 replies; 5+ messages in thread
From: Leo Martins @ 2025-12-13  1:28 UTC (permalink / raw)
  To: Filipe Manana; +Cc: linux-btrfs, kernel-team, kernel test robot

On Fri, 12 Dec 2025 09:32:06 +0000 Filipe Manana <fdmanana@kernel.org> wrote:

> On Fri, Dec 12, 2025 at 1:01 AM Leo Martins <loemra.dev@gmail.com> wrote:
> >
> > On Wed, 3 Dec 2025 10:50:17 +0000 Filipe Manana <fdmanana@kernel.org> wrote:
> >
> > > On Tue, Dec 2, 2025 at 9:21 PM Leo Martins <loemra.dev@gmail.com> wrote:
> > > >
> > > > Previously, btrfs_get_or_create_delayed_node sets the delayed_node's
> > > > refcount before acquiring the root->delayed_nodes lock.
> > > > Commit e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
> > > > moves refcount_set inside the critical section which means
> > > > there is no longer a memory barrier between setting the refcount and
> > > > setting btrfs_inode->delayed_node = node.
> > > >
> > > > This allows btrfs_get_or_create_delayed_node to set
> > > > btrfs_inode->delayed_node before setting the refcount.
> > > > A different thread is then able to read and increase the refcount
> > > > of btrfs_inode->delayed_node leading to a refcounting bug and
> > > > a use-after-free warning.
> > > >
> > > > The fix is to move refcount_set back to where it was to take
> > > > advantage of the implicit memory barrier provided by lock
> > > > acquisition.
> > > >
> > > > Fixes: e8513c012de7 ("btrfs: implement ref_tracker for delayed_nodes")
> > > > Reported-by: kernel test robot <oliver.sang@intel.com>
> > > > Closes: https://lore.kernel.org/oe-lkp/202511262228.6dda231e-lkp@intel.com
> > > > Tested-by: kernel test robot <oliver.sang@intel.com>
> > > > Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> > > > ---
> > > >  fs/btrfs/delayed-inode.c | 32 +++++++++++++++++---------------
> > > >  1 file changed, 17 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> > > > index 364814642a91..81922556379d 100644
> > > > --- a/fs/btrfs/delayed-inode.c
> > > > +++ b/fs/btrfs/delayed-inode.c
> > > > @@ -152,37 +152,39 @@ static struct btrfs_delayed_node *btrfs_get_or_create_delayed_node(
> > > >                 return ERR_PTR(-ENOMEM);
> > > >         btrfs_init_delayed_node(node, root, ino);
> > > >
> > > > +       /* Cached in the inode and can be accessed. */
> > > > +       refcount_set(&node->refs, 2);
> > > > +       btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
> > > > +       btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
> > >
> > > Now that we do these allocations outside the spinlock (xarray's lock),
> > > we can stop using GFP_ATOMIC and use GFP_NOFS.
> > >
> > > Otherwise it looks good, thanks.
> >
> > Did you want me to send out a v3 for this change?
> 
> You can make it either in a v3 or as a separate patch, I don't have a
> strong preference for this.
> 
> Either way:
> 
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> 
> (Applies both to this, a possible v3 and a standalone patch for
> replacing GFP_ATOMIC with GFP_NOFS).
> 
> If you can't push to for-next, let me know and I can do it.

Just sent out a v3. I can't push to for-next, if you could that
would be great.

Thanks.

> 
> Thanks.
> 
> >
> > >
> > > > +
> > > >         /* Allocate and reserve the slot, from now it can return a NULL from xa_load(). */
> > > >         ret = xa_reserve(&root->delayed_nodes, ino, GFP_NOFS);
> > > > -       if (ret == -ENOMEM) {
> > > > -               btrfs_delayed_node_ref_tracker_dir_exit(node);
> > > > -               kmem_cache_free(delayed_node_cache, node);
> > > > -               return ERR_PTR(-ENOMEM);
> > > > -       }
> > > > +       if (ret == -ENOMEM)
> > > > +               goto cleanup;
> > > > +
> > > >         xa_lock(&root->delayed_nodes);
> > > >         ptr = xa_load(&root->delayed_nodes, ino);
> > > >         if (ptr) {
> > > >                 /* Somebody inserted it, go back and read it. */
> > > >                 xa_unlock(&root->delayed_nodes);
> > > > -               btrfs_delayed_node_ref_tracker_dir_exit(node);
> > > > -               kmem_cache_free(delayed_node_cache, node);
> > > > -               node = NULL;
> > > > -               goto again;
> > > > +               goto cleanup;
> > > >         }
> > > >         ptr = __xa_store(&root->delayed_nodes, ino, node, GFP_ATOMIC);
> > > >         ASSERT(xa_err(ptr) != -EINVAL);
> > > >         ASSERT(xa_err(ptr) != -ENOMEM);
> > > >         ASSERT(ptr == NULL);
> > > > -
> > > > -       /* Cached in the inode and can be accessed. */
> > > > -       refcount_set(&node->refs, 2);
> > > > -       btrfs_delayed_node_ref_tracker_alloc(node, tracker, GFP_ATOMIC);
> > > > -       btrfs_delayed_node_ref_tracker_alloc(node, &node->inode_cache_tracker, GFP_ATOMIC);
> > > > -
> > > >         btrfs_inode->delayed_node = node;
> > > >         xa_unlock(&root->delayed_nodes);
> > > >
> > > >         return node;
> > > > +cleanup:
> > > > +       btrfs_delayed_node_ref_tracker_free(node, tracker);
> > > > +       btrfs_delayed_node_ref_tracker_free(node, &node->inode_cache_tracker);
> > > > +       btrfs_delayed_node_ref_tracker_dir_exit(node);
> > > > +       kmem_cache_free(delayed_node_cache, node);
> > > > +       if (ret)
> > > > +               return ERR_PTR(ret);
> > > > +       goto again;
> > > >  }
> > > >
> > > >  /*
> > > > --
> > > > 2.47.3
> > > >
> > > >

Sent using hkml (https://github.com/sjp38/hackermail)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-13  1:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 21:21 [PATCH v2] btrfs: fix use-after-free warning in btrfs_get_or_create_delayed_node Leo Martins
2025-12-03 10:50 ` Filipe Manana
2025-12-12  1:01   ` Leo Martins
2025-12-12  9:32     ` Filipe Manana
2025-12-13  1:28       ` Leo Martins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox