* [PATCH] hfs: validate bitmap record offset in hfs_bmap_alloc
@ 2026-04-18 13:39 Tristan Madani
2026-04-22 23:05 ` Viacheslav Dubeyko
0 siblings, 1 reply; 2+ messages in thread
From: Tristan Madani @ 2026-04-18 13:39 UTC (permalink / raw)
To: slava, glaubitz, frank.li
Cc: linux-fsdevel, akpm, stable, syzbot+a19ca73b21fe8bc69101,
Tristan Madani
hfs_bmap_alloc() retrieves the bitmap record from the header node
using hfs_brec_lenoff() but does not validate the returned offset
and length before using them to compute page pointers. On a crafted
HFS image with corrupted B-tree data, the offset can exceed the node
size, causing an out-of-bounds page access.
Additionally, when the bitmap has node 0 bit incorrectly unset,
hfs_bmap_alloc() calls hfs_bnode_create(tree, 0) for the already-
hashed header node, triggering a WARN_ON and returning the existing
node without incrementing its reference count.
Port the fixes already applied to HFS+ (commits d8a73cc46c84 and
738d5a51864e) to the HFS side:
1. Move is_bnode_offset_valid() and check_and_correct_requested_length()
from bnode.c to btree.h so they can be used by btree.c.
2. Validate the record offset and length in hfs_bmap_alloc() before
computing page pointers, preventing out-of-bounds access.
3. Return ERR_PTR(-EEXIST) from hfs_bnode_create() when the node is
already hashed, properly signaling filesystem corruption to callers.
Reported-by: syzbot+a19ca73b21fe8bc69101@syzkaller.appspotmail.com
Tested-by: syzbot+a19ca73b21fe8bc69101@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=a19ca73b21fe8bc69101
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
fs/hfs/bnode.c | 44 +-------------------------------------------
fs/hfs/btree.c | 6 ++++++
fs/hfs/btree.h | 42 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 49 insertions(+), 43 deletions(-)
diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
index 13d58c51fc46b..26c2e65ab5935 100644
--- a/fs/hfs/bnode.c
+++ b/fs/hfs/bnode.c
@@ -15,48 +15,6 @@
#include "btree.h"
-static inline
-bool is_bnode_offset_valid(struct hfs_bnode *node, u32 off)
-{
- bool is_valid = off < node->tree->node_size;
-
- if (!is_valid) {
- pr_err("requested invalid offset: "
- "NODE: id %u, type %#x, height %u, "
- "node_size %u, offset %u\n",
- node->this, node->type, node->height,
- node->tree->node_size, off);
- }
-
- return is_valid;
-}
-
-static inline
-u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
-{
- unsigned int node_size;
-
- if (!is_bnode_offset_valid(node, off))
- return 0;
-
- node_size = node->tree->node_size;
-
- if ((off + len) > node_size) {
- u32 new_len = node_size - off;
-
- pr_err("requested length has been corrected: "
- "NODE: id %u, type %#x, height %u, "
- "node_size %u, offset %u, "
- "requested_len %u, corrected_len %u\n",
- node->this, node->type, node->height,
- node->tree->node_size, off, len, new_len);
-
- return new_len;
- }
-
- return len;
-}
-
void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
{
struct page *page;
@@ -518,7 +476,7 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num)
if (node) {
pr_crit("new node %u already hashed?\n", num);
WARN_ON(1);
- return node;
+ return ERR_PTR(-EEXIST);
}
node = __hfs_bnode_create(tree, num);
if (!node)
diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
index 2eb37a2f64e86..e8bc24c8baf1a 100644
--- a/fs/hfs/btree.c
+++ b/fs/hfs/btree.c
@@ -304,6 +304,12 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
len = hfs_brec_lenoff(node, 2, &off16);
off = off16;
+ if (!is_bnode_offset_valid(node, off)) {
+ hfs_bnode_put(node);
+ return ERR_PTR(-EIO);
+ }
+ len = check_and_correct_requested_length(node, off, len);
+
off += node->page_offset;
pagep = node->page + (off >> PAGE_SHIFT);
data = kmap_local_page(*pagep);
diff --git a/fs/hfs/btree.h b/fs/hfs/btree.h
index 99be858b24465..6032b14b1639d 100644
--- a/fs/hfs/btree.h
+++ b/fs/hfs/btree.h
@@ -85,6 +85,48 @@ struct hfs_find_data {
};
+static inline
+bool is_bnode_offset_valid(struct hfs_bnode *node, u32 off)
+{
+ bool is_valid = off < node->tree->node_size;
+
+ if (!is_valid) {
+ pr_err("requested invalid offset: "
+ "NODE: id %u, type %#x, height %u, "
+ "node_size %u, offset %u\n",
+ node->this, node->type, node->height,
+ node->tree->node_size, off);
+ }
+
+ return is_valid;
+}
+
+static inline
+u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
+{
+ unsigned int node_size;
+
+ if (!is_bnode_offset_valid(node, off))
+ return 0;
+
+ node_size = node->tree->node_size;
+
+ if ((off + len) > node_size) {
+ u32 new_len = node_size - off;
+
+ pr_err("requested length has been corrected: "
+ "NODE: id %u, type %#x, height %u, "
+ "node_size %u, offset %u, "
+ "requested_len %u, corrected_len %u\n",
+ node->this, node->type, node->height,
+ node->tree->node_size, off, len, new_len);
+
+ return new_len;
+ }
+
+ return len;
+}
+
/* btree.c */
extern struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id,
btree_keycmp keycmp);
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] hfs: validate bitmap record offset in hfs_bmap_alloc
2026-04-18 13:39 [PATCH] hfs: validate bitmap record offset in hfs_bmap_alloc Tristan Madani
@ 2026-04-22 23:05 ` Viacheslav Dubeyko
0 siblings, 0 replies; 2+ messages in thread
From: Viacheslav Dubeyko @ 2026-04-22 23:05 UTC (permalink / raw)
To: Tristan Madani, slava, glaubitz, frank.li
Cc: linux-fsdevel, akpm, stable, syzbot+a19ca73b21fe8bc69101,
Tristan Madani
On Sat, 2026-04-18 at 13:39 +0000, Tristan Madani wrote:
> hfs_bmap_alloc() retrieves the bitmap record from the header node
> using hfs_brec_lenoff() but does not validate the returned offset
> and length before using them to compute page pointers. On a crafted
> HFS image with corrupted B-tree data, the offset can exceed the node
> size, causing an out-of-bounds page access.
>
> Additionally, when the bitmap has node 0 bit incorrectly unset,
> hfs_bmap_alloc() calls hfs_bnode_create(tree, 0) for the already-
> hashed header node, triggering a WARN_ON and returning the existing
> node without incrementing its reference count.
>
> Port the fixes already applied to HFS+ (commits d8a73cc46c84 and
> 738d5a51864e) to the HFS side:
>
> 1. Move is_bnode_offset_valid() and check_and_correct_requested_length()
> from bnode.c to btree.h so they can be used by btree.c.
>
> 2. Validate the record offset and length in hfs_bmap_alloc() before
> computing page pointers, preventing out-of-bounds access.
>
> 3. Return ERR_PTR(-EEXIST) from hfs_bnode_create() when the node is
> already hashed, properly signaling filesystem corruption to callers.
I assume that we have the second version of the patch again. Am I correct?
> Reported-by: syzbot+a19ca73b21fe8bc69101@syzkaller.appspotmail.com
> Tested-by: syzbot+a19ca73b21fe8bc69101@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=a19ca73b21fe8bc69101
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> fs/hfs/bnode.c | 44 +-------------------------------------------
> fs/hfs/btree.c | 6 ++++++
> fs/hfs/btree.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 49 insertions(+), 43 deletions(-)
>
> diff --git a/fs/hfs/bnode.c b/fs/hfs/bnode.c
> index 13d58c51fc46b..26c2e65ab5935 100644
> --- a/fs/hfs/bnode.c
> +++ b/fs/hfs/bnode.c
> @@ -15,48 +15,6 @@
>
> #include "btree.h"
>
> -static inline
> -bool is_bnode_offset_valid(struct hfs_bnode *node, u32 off)
> -{
> - bool is_valid = off < node->tree->node_size;
> -
> - if (!is_valid) {
> - pr_err("requested invalid offset: "
> - "NODE: id %u, type %#x, height %u, "
> - "node_size %u, offset %u\n",
> - node->this, node->type, node->height,
> - node->tree->node_size, off);
> - }
> -
> - return is_valid;
> -}
> -
> -static inline
> -u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
> -{
> - unsigned int node_size;
> -
> - if (!is_bnode_offset_valid(node, off))
> - return 0;
> -
> - node_size = node->tree->node_size;
> -
> - if ((off + len) > node_size) {
> - u32 new_len = node_size - off;
> -
> - pr_err("requested length has been corrected: "
> - "NODE: id %u, type %#x, height %u, "
> - "node_size %u, offset %u, "
> - "requested_len %u, corrected_len %u\n",
> - node->this, node->type, node->height,
> - node->tree->node_size, off, len, new_len);
> -
> - return new_len;
> - }
> -
> - return len;
> -}
> -
> void hfs_bnode_read(struct hfs_bnode *node, void *buf, u32 off, u32 len)
> {
> struct page *page;
> @@ -518,7 +476,7 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num)
> if (node) {
> pr_crit("new node %u already hashed?\n", num);
> WARN_ON(1);
> - return node;
> + return ERR_PTR(-EEXIST);
> }
> node = __hfs_bnode_create(tree, num);
> if (!node)
> diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
> index 2eb37a2f64e86..e8bc24c8baf1a 100644
> --- a/fs/hfs/btree.c
> +++ b/fs/hfs/btree.c
> @@ -304,6 +304,12 @@ struct hfs_bnode *hfs_bmap_alloc(struct hfs_btree *tree)
> len = hfs_brec_lenoff(node, 2, &off16);
> off = off16;
>
> + if (!is_bnode_offset_valid(node, off)) {
> + hfs_bnode_put(node);
> + return ERR_PTR(-EIO);
> + }
> + len = check_and_correct_requested_length(node, off, len);
> +
> off += node->page_offset;
> pagep = node->page + (off >> PAGE_SHIFT);
> data = kmap_local_page(*pagep);
> diff --git a/fs/hfs/btree.h b/fs/hfs/btree.h
> index 99be858b24465..6032b14b1639d 100644
> --- a/fs/hfs/btree.h
> +++ b/fs/hfs/btree.h
> @@ -85,6 +85,48 @@ struct hfs_find_data {
> };
>
>
> +static inline
> +bool is_bnode_offset_valid(struct hfs_bnode *node, u32 off)
> +{
> + bool is_valid = off < node->tree->node_size;
> +
> + if (!is_valid) {
> + pr_err("requested invalid offset: "
> + "NODE: id %u, type %#x, height %u, "
> + "node_size %u, offset %u\n",
> + node->this, node->type, node->height,
> + node->tree->node_size, off);
> + }
> +
> + return is_valid;
> +}
> +
> +static inline
> +u32 check_and_correct_requested_length(struct hfs_bnode *node, u32 off, u32 len)
> +{
> + unsigned int node_size;
> +
> + if (!is_bnode_offset_valid(node, off))
> + return 0;
> +
> + node_size = node->tree->node_size;
> +
> + if ((off + len) > node_size) {
> + u32 new_len = node_size - off;
> +
> + pr_err("requested length has been corrected: "
> + "NODE: id %u, type %#x, height %u, "
> + "node_size %u, offset %u, "
> + "requested_len %u, corrected_len %u\n",
> + node->this, node->type, node->height,
> + node->tree->node_size, off, len, new_len);
> +
> + return new_len;
> + }
> +
> + return len;
> +}
> +
> /* btree.c */
> extern struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id,
> btree_keycmp keycmp);
This modification makes sense but I mean the porting of slightly different
functionality. Are you working on it? Or have you missed my point?
Thanks,
Slava.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-04-22 23:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-18 13:39 [PATCH] hfs: validate bitmap record offset in hfs_bmap_alloc Tristan Madani
2026-04-22 23:05 ` Viacheslav Dubeyko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox