fs/hfsplus/bnode.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
hfs_bnode_put() does not support deallocating a newly created btree
head node; therefore, regardless of whether hfsplus_bnode_find() succeeds
or fails, it cannot effectively reclaim the memory allocated for a newly
created head node.
When finding a head node, if the node is a newly created one, we can use
hfs_bnode_free() to reclaim its memory.
[1]
BUG: memory leak
unreferenced object 0xffff88811cabc840 (size 96):
backtrace (crc 3e2dadb7):
__hfs_bnode_create+0x59/0x310 fs/hfsplus/bnode.c:469
hfsplus_bnode_find+0x13e/0x580 fs/hfsplus/bnode.c:547
hfsplus_btree_open+0x2fa/0x6d0 fs/hfsplus/btree.c:382
hfsplus_fill_super+0x272/0x880 fs/hfsplus/super.c:548
Fixes: 8ad2c6a36ac4 ("hfsplus: validate b-tree node 0 bitmap at mount time")
Reported-by: syzbot+98547b0428b6a6a3467c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=98547b0428b6a6a3467c
Tested-by: syzbot+98547b0428b6a6a3467c@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
fs/hfsplus/bnode.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index f8b5a8ae58ff..65902104882a 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -598,14 +598,18 @@ struct hfs_bnode *hfs_bnode_find(struct hfs_btree *tree, u32 num)
if (key_size >= entry_size || key_size & 1)
goto node_error;
}
- clear_bit(HFS_BNODE_NEW, &node->flags);
- wake_up(&node->lock_wq);
+ if (num != HFSPLUS_TREE_HEAD) {
+ clear_bit(HFS_BNODE_NEW, &node->flags);
+ wake_up(&node->lock_wq);
+ }
return node;
node_error:
set_bit(HFS_BNODE_ERROR, &node->flags);
- clear_bit(HFS_BNODE_NEW, &node->flags);
- wake_up(&node->lock_wq);
+ if (num != HFSPLUS_TREE_HEAD) {
+ clear_bit(HFS_BNODE_NEW, &node->flags);
+ wake_up(&node->lock_wq);
+ }
hfs_bnode_put(node);
return ERR_PTR(-EIO);
}
@@ -694,6 +698,10 @@ void hfs_bnode_put(struct hfs_bnode *node)
hfs_bnode_free(node);
return;
}
+ if (test_bit(HFS_BNODE_NEW, &node->flags)) {
+ hfs_bnode_unhash(node);
+ hfs_bnode_free(node);
+ }
spin_unlock(&tree->hash_lock);
}
}
--
2.43.0On Fri, 2026-04-17 at 14:58 +0800, Edward Adam Davis wrote:
> hfs_bnode_put() does not support deallocating a newly created btree
> head node; therefore, regardless of whether hfsplus_bnode_find() succeeds
> or fails, it cannot effectively reclaim the memory allocated for a newly
> created head node.
>
> When finding a head node, if the node is a newly created one, we can use
> hfs_bnode_free() to reclaim its memory.
>
> [1]
> BUG: memory leak
> unreferenced object 0xffff88811cabc840 (size 96):
> backtrace (crc 3e2dadb7):
> __hfs_bnode_create+0x59/0x310 fs/hfsplus/bnode.c:469
> hfsplus_bnode_find+0x13e/0x580 fs/hfsplus/bnode.c:547
> hfsplus_btree_open+0x2fa/0x6d0 fs/hfsplus/btree.c:382
> hfsplus_fill_super+0x272/0x880 fs/hfsplus/super.c:548
I need slightly more detailed and clear explanation how the issue has been
happened and why we need to fix it in a such way. Currently, I don't have the
complete picture how this happened. I am sure that you have the complete
picture. :) Could you please share what you have in your mind? :) Don't hide the
wisdom. ;)
Thanks,
Slava.
>
> Fixes: 8ad2c6a36ac4 ("hfsplus: validate b-tree node 0 bitmap at mount time")
> Reported-by: syzbot+98547b0428b6a6a3467c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=98547b0428b6a6a3467c
> Tested-by: syzbot+98547b0428b6a6a3467c@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> fs/hfsplus/bnode.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index f8b5a8ae58ff..65902104882a 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -598,14 +598,18 @@ struct hfs_bnode *hfs_bnode_find(struct hfs_btree *tree, u32 num)
> if (key_size >= entry_size || key_size & 1)
> goto node_error;
> }
> - clear_bit(HFS_BNODE_NEW, &node->flags);
> - wake_up(&node->lock_wq);
> + if (num != HFSPLUS_TREE_HEAD) {
> + clear_bit(HFS_BNODE_NEW, &node->flags);
> + wake_up(&node->lock_wq);
> + }
> return node;
>
> node_error:
> set_bit(HFS_BNODE_ERROR, &node->flags);
> - clear_bit(HFS_BNODE_NEW, &node->flags);
> - wake_up(&node->lock_wq);
> + if (num != HFSPLUS_TREE_HEAD) {
> + clear_bit(HFS_BNODE_NEW, &node->flags);
> + wake_up(&node->lock_wq);
> + }
> hfs_bnode_put(node);
> return ERR_PTR(-EIO);
> }
> @@ -694,6 +698,10 @@ void hfs_bnode_put(struct hfs_bnode *node)
> hfs_bnode_free(node);
> return;
> }
> + if (test_bit(HFS_BNODE_NEW, &node->flags)) {
> + hfs_bnode_unhash(node);
> + hfs_bnode_free(node);
> + }
> spin_unlock(&tree->hash_lock);
> }
> }
On Fri, 17 Apr 2026 15:03:17 -0700, Viacheslav Dubeyko wrote:
> > hfs_bnode_put() does not support deallocating a newly created btree
> > head node; therefore, regardless of whether hfsplus_bnode_find() succeeds
> > or fails, it cannot effectively reclaim the memory allocated for a newly
> > created head node.
> >
> > When finding a head node, if the node is a newly created one, we can use
> > hfs_bnode_free() to reclaim its memory.
> >
> > [1]
> > BUG: memory leak
> > unreferenced object 0xffff88811cabc840 (size 96):
> > backtrace (crc 3e2dadb7):
> > __hfs_bnode_create+0x59/0x310 fs/hfsplus/bnode.c:469
> > hfsplus_bnode_find+0x13e/0x580 fs/hfsplus/bnode.c:547
> > hfsplus_btree_open+0x2fa/0x6d0 fs/hfsplus/btree.c:382
> > hfsplus_fill_super+0x272/0x880 fs/hfsplus/super.c:548
>
> I need slightly more detailed and clear explanation how the issue has been
> happened and why we need to fix it in a such way. Currently, I don't have the
> complete picture how this happened. I am sure that you have the complete
> picture. :) Could you please share what you have in your mind? :) Don't hide the
> wisdom. ;)
fc_mount()
vfs_get_tree()
get_tree_bdev_flags()
hfsplus_fill_super()
hfsplus_btree_open()
hfsplus_bnode_find(tree, HFSPLUS_TREE_HEAD)
__hfs_bnode_create()
...
node->this = cnid; // cnid is HFSPLUS_TREE_HEAD, it is 0
...
...
desc = (struct hfs_bnode_desc *)(kmap_local_page(node->page[0])
node->type = desc->type;
switch (node->type) {
...
default: // because node->type is 127, so run here
goto node_error;
...
hfs_bnode_put()
...
if (test_bit(HFS_BNODE_DELETED, &node->flags)) { /* head node not set HFS_BNODE_DELETED flag
* so, the node will never be freed.
* Furthermore, even if the node were marked
* with the HFS_BNODE_DELETED flag, hfs_bmap_free()
* would still trigger BUG_ON(!node->this).
*/
Edward
BR
On Sat, 2026-04-18 at 17:37 +0800, Edward Adam Davis wrote:
> On Fri, 17 Apr 2026 15:03:17 -0700, Viacheslav Dubeyko wrote:
> > > hfs_bnode_put() does not support deallocating a newly created btree
> > > head node; therefore, regardless of whether hfsplus_bnode_find() succeeds
> > > or fails, it cannot effectively reclaim the memory allocated for a newly
> > > created head node.
> > >
> > > When finding a head node, if the node is a newly created one, we can use
> > > hfs_bnode_free() to reclaim its memory.
> > >
> > > [1]
> > > BUG: memory leak
> > > unreferenced object 0xffff88811cabc840 (size 96):
> > > backtrace (crc 3e2dadb7):
> > > __hfs_bnode_create+0x59/0x310 fs/hfsplus/bnode.c:469
> > > hfsplus_bnode_find+0x13e/0x580 fs/hfsplus/bnode.c:547
> > > hfsplus_btree_open+0x2fa/0x6d0 fs/hfsplus/btree.c:382
> > > hfsplus_fill_super+0x272/0x880 fs/hfsplus/super.c:548
> >
> > I need slightly more detailed and clear explanation how the issue has been
> > happened and why we need to fix it in a such way. Currently, I don't have the
> > complete picture how this happened. I am sure that you have the complete
> > picture. :) Could you please share what you have in your mind? :) Don't hide the
> > wisdom. ;)
> fc_mount()
> vfs_get_tree()
> get_tree_bdev_flags()
> hfsplus_fill_super()
We have multiple calls of hfsplus_btree_open(). Which particular b-tree do you
mean here?
> hfsplus_btree_open()
I assume that you mean hfs_btree_open().
> hfsplus_bnode_find(tree, HFSPLUS_TREE_HEAD)
>
We call hfs_bnode_find() here.
>
> __hfs_bnode_create()
> ...
> node->this = cnid; // cnid is HFSPLUS_TREE_HEAD, it is 0
> ...
> ...
> desc = (struct hfs_bnode_desc *)(kmap_local_page(node->page[0])
> node->type = desc->type;
> switch (node->type) {
> ...
> default: // because node->type is 127, so run here
> goto node_error;
I think, maybe, we need to add some error message(s) when some error is
happened. What do you think?
> ...
> hfs_bnode_put()
> ...
> if (test_bit(HFS_BNODE_DELETED, &node->flags)) { /* head node not set HFS_BNODE_DELETED flag
> * so, the node will never be freed.
> * Furthermore, even if the node were marked
> * with the HFS_BNODE_DELETED flag, hfs_bmap_free()
> * would still trigger BUG_ON(!node->this).
> */
node_error:
set_bit(HFS_BNODE_ERROR, &node->flags);
- clear_bit(HFS_BNODE_NEW, &node->flags);
- wake_up(&node->lock_wq);
+ if (num != HFSPLUS_TREE_HEAD) {
Why do we need to distinguish is it HFSPLUS_TREE_HEAD or not? I don't see any
difference in node processing.
+ clear_bit(HFS_BNODE_NEW, &node->flags);
I think that we don't need to clear the HFS_BNODE_NEW. Because, we need to
analyze HFS_BNODE_ERROR in hfs_bnode_put().
+ wake_up(&node->lock_wq);
I assume that wake_up() should be called anyway. Why have you removed this call
for HFSPLUS_TREE_HEAD case?
+ }
hfs_bnode_put(node);
return ERR_PTR(-EIO);
@@ -694,6 +698,10 @@ void hfs_bnode_put(struct hfs_bnode *node)
hfs_bnode_free(node);
return;
}
+ if (test_bit(HFS_BNODE_NEW, &node->flags)) {
I suggest to analyze HFS_BNODE_ERROR here.
+ hfs_bnode_unhash(node);
+ hfs_bnode_free(node);
+ }
@@ -598,14 +598,18 @@ struct hfs_bnode *hfs_bnode_find(struct hfs_btree *tree,
u32 num)
if (key_size >= entry_size || key_size & 1)
goto node_error;
}
- clear_bit(HFS_BNODE_NEW, &node->flags);
- wake_up(&node->lock_wq);
+ if (num != HFSPLUS_TREE_HEAD) {
+ clear_bit(HFS_BNODE_NEW, &node->flags);
+ wake_up(&node->lock_wq);
What the point in this code? Why have you change the logic here?
+ }
return node;
And, could you please add these new details into the commit message?
Thanks,
Slava.
Hi Viacheslav Dubeyko,
Apologies for the late reply.
On Wed, 22 Apr 2026 11:19:10 -0700, Viacheslav Dubeyko wrote:
> On Sat, 2026-04-18 at 17:37 +0800, Edward Adam Davis wrote:
> > On Fri, 17 Apr 2026 15:03:17 -0700, Viacheslav Dubeyko wrote:
> > > > hfs_bnode_put() does not support deallocating a newly created btree
> > > > head node; therefore, regardless of whether hfsplus_bnode_find() succeeds
> > > > or fails, it cannot effectively reclaim the memory allocated for a newly
> > > > created head node.
> > > >
> > > > When finding a head node, if the node is a newly created one, we can use
> > > > hfs_bnode_free() to reclaim its memory.
> > > >
> > > > [1]
> > > > BUG: memory leak
> > > > unreferenced object 0xffff88811cabc840 (size 96):
> > > > backtrace (crc 3e2dadb7):
> > > > __hfs_bnode_create+0x59/0x310 fs/hfsplus/bnode.c:469
> > > > hfsplus_bnode_find+0x13e/0x580 fs/hfsplus/bnode.c:547
> > > > hfsplus_btree_open+0x2fa/0x6d0 fs/hfsplus/btree.c:382
> > > > hfsplus_fill_super+0x272/0x880 fs/hfsplus/super.c:548
> > >
> > > I need slightly more detailed and clear explanation how the issue has been
> > > happened and why we need to fix it in a such way. Currently, I don't have the
> > > complete picture how this happened. I am sure that you have the complete
> > > picture. :) Could you please share what you have in your mind? :) Don't hide the
> > > wisdom. ;)
> > fc_mount()
> > vfs_get_tree()
> > get_tree_bdev_flags()
> > hfsplus_fill_super()
>
> We have multiple calls of hfsplus_btree_open(). Which particular b-tree do you
> mean here?
>
> > hfsplus_btree_open()
>
> I assume that you mean hfs_btree_open().
>
> > hfsplus_bnode_find(tree, HFSPLUS_TREE_HEAD)
> >
>
> We call hfs_bnode_find() here.
>
> >
> > __hfs_bnode_create()
> > ...
> > node->this = cnid; // cnid is HFSPLUS_TREE_HEAD, it is 0
> > ...
> > ...
> > desc = (struct hfs_bnode_desc *)(kmap_local_page(node->page[0])
> > node->type = desc->type;
> > switch (node->type) {
> > ...
> > default: // because node->type is 127, so run here
Following "default," the comment I added clearly indicates that the
issue stems from the `type` value being 127.
> > goto node_error;
>
> I think, maybe, we need to add some error message(s) when some error is
> happened. What do you think?
>
> > ...
> > hfs_bnode_put()
> > ...
> > if (test_bit(HFS_BNODE_DELETED, &node->flags)) { /* head node not set HFS_BNODE_DELETED flag
> > * so, the node will never be freed.
> > * Furthermore, even if the node were marked
> > * with the HFS_BNODE_DELETED flag, hfs_bmap_free()
> > * would still trigger BUG_ON(!node->this).
Please note the comments I added above.
> > */
>
> node_error:
> set_bit(HFS_BNODE_ERROR, &node->flags);
> - clear_bit(HFS_BNODE_NEW, &node->flags);
> - wake_up(&node->lock_wq);
> + if (num != HFSPLUS_TREE_HEAD) {
>
> Why do we need to distinguish is it HFSPLUS_TREE_HEAD or not? I don't see any
> difference in node processing.
>
> + clear_bit(HFS_BNODE_NEW, &node->flags);
>
> I think that we don't need to clear the HFS_BNODE_NEW. Because, we need to
> analyze HFS_BNODE_ERROR in hfs_bnode_put().
>
> + wake_up(&node->lock_wq);
>
> I assume that wake_up() should be called anyway. Why have you removed this call
> for HFSPLUS_TREE_HEAD case?
Based on my analysis, hfs_bnode_find() does not support searching for
HFSPLUS_TREE_HEAD.
>
> + }
> hfs_bnode_put(node);
> return ERR_PTR(-EIO);
>
> @@ -694,6 +698,10 @@ void hfs_bnode_put(struct hfs_bnode *node)
> hfs_bnode_free(node);
> return;
> }
> + if (test_bit(HFS_BNODE_NEW, &node->flags)) {
>
> I suggest to analyze HFS_BNODE_ERROR here.
Only the new head node will reach this point, no further analysis is
required.
>
> + hfs_bnode_unhash(node);
> + hfs_bnode_free(node);
> + }
>
> @@ -598,14 +598,18 @@ struct hfs_bnode *hfs_bnode_find(struct hfs_btree *tree,
> u32 num)
> if (key_size >= entry_size || key_size & 1)
> goto node_error;
> }
> - clear_bit(HFS_BNODE_NEW, &node->flags);
> - wake_up(&node->lock_wq);
> + if (num != HFSPLUS_TREE_HEAD) {
> + clear_bit(HFS_BNODE_NEW, &node->flags);
> + wake_up(&node->lock_wq);
>
> What the point in this code? Why have you change the logic here?
To support tree head.
>
> + }
> return node;
>
> And, could you please add these new details into the commit message?
OK.
Edward
BR
© 2016 - 2026 Red Hat, Inc.