[PATCH] hfs: clear offset and space out of valid records in b-tree node

[PATCH] hfs: clear offset and space out of valid records in b-tree node

[Viacheslav Dubeyko]

Currently, hfs_brec_remove() executes moving records
towards the location of deleted record and it updates
offsets of moved records. However, the hfs_brec_remove()
logic ignores the "mess" of b-tree node's free space and
it doesn't touch the offsets out of records number.
Potentially, it could confuse fsck or driver logic or
to be a reason of potential corruption cases.

This patch reworks the logic of hfs_brec_remove()
by means of clearing freed space of b-tree node
after the records moving. And it clear the last
offset that keeping old location of free space
because now the offset before this one is keeping
the actual offset to the free space after the record
deletion.

Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
---
 fs/hfs/brec.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/fs/hfs/brec.c b/fs/hfs/brec.c
index 896396554bcc..b01db1fae147 100644
--- a/fs/hfs/brec.c
+++ b/fs/hfs/brec.c
@@ -179,6 +179,7 @@ int hfs_brec_remove(struct hfs_find_data *fd)
 	struct hfs_btree *tree;
 	struct hfs_bnode *node, *parent;
 	int end_off, rec_off, data_off, size;
+	int src, dst, len;
 
 	tree = fd->tree;
 	node = fd->bnode;
@@ -208,10 +209,14 @@ int hfs_brec_remove(struct hfs_find_data *fd)
 	}
 	hfs_bnode_write_u16(node, offsetof(struct hfs_bnode_desc, num_recs), node->num_recs);
 
-	if (rec_off == end_off)
-		goto skip;
 	size = fd->keylength + fd->entrylength;
 
+	if (rec_off == end_off) {
+		src = fd->keyoffset;
+		hfs_bnode_clear(node, src, size);
+		goto skip;
+	}
+
 	do {
 		data_off = hfs_bnode_read_u16(node, rec_off);
 		hfs_bnode_write_u16(node, rec_off + 2, data_off - size);
@@ -219,9 +224,23 @@ int hfs_brec_remove(struct hfs_find_data *fd)
 	} while (rec_off >= end_off);
 
 	/* fill hole */
-	hfs_bnode_move(node, fd->keyoffset, fd->keyoffset + size,
-		       data_off - fd->keyoffset - size);
+	dst = fd->keyoffset;
+	src = fd->keyoffset + size;
+	len = data_off - src;
+
+	hfs_bnode_move(node, dst, src, len);
+
+	src = dst + len;
+	len = data_off - src;
+
+	hfs_bnode_clear(node, src, len);
+
 skip:
+	/*
+	 * Remove the obsolete offset to free space.
+	 */
+	hfs_bnode_write_u16(node, end_off, 0);
+
 	hfs_bnode_dump(node);
 	if (!fd->record)
 		hfs_brec_update_parent(fd);
-- 
2.43.0

Re: [PATCH] hfs: clear offset and space out of valid records in b-tree node

[Yangtao Li]

Hi Slava,

在 2025/8/16 03:49, Viacheslav Dubeyko 写道:
> Currently, hfs_brec_remove() executes moving records
> towards the location of deleted record and it updates
> offsets of moved records. However, the hfs_brec_remove()
> logic ignores the "mess" of b-tree node's free space and
> it doesn't touch the offsets out of records number.
> Potentially, it could confuse fsck or driver logic or
> to be a reason of potential corruption cases.

Patch looks good, and I don't object to it.
But I don't know what dose it mean

'it could confuse fsck or driver logic or to be a reason of potential 
corruption cases.'


What cases?

> 
> This patch reworks the logic of hfs_brec_remove()
> by means of clearing freed space of b-tree node
> after the records moving. And it clear the last
> offset that keeping old location of free space
> because now the offset before this one is keeping
> the actual offset to the free space after the record
> deletion.
> 
> Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
> cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> cc: Yangtao Li <frank.li@vivo.com>
> cc: linux-fsdevel@vger.kernel.org
> ---
>   fs/hfs/brec.c | 27 +++++++++++++++++++++++----
>   1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/hfs/brec.c b/fs/hfs/brec.c
> index 896396554bcc..b01db1fae147 100644
> --- a/fs/hfs/brec.c
> +++ b/fs/hfs/brec.c
> @@ -179,6 +179,7 @@ int hfs_brec_remove(struct hfs_find_data *fd)
>   	struct hfs_btree *tree;
>   	struct hfs_bnode *node, *parent;
>   	int end_off, rec_off, data_off, size;
> +	int src, dst, len;
>   
>   	tree = fd->tree;
>   	node = fd->bnode;
> @@ -208,10 +209,14 @@ int hfs_brec_remove(struct hfs_find_data *fd)
>   	}
>   	hfs_bnode_write_u16(node, offsetof(struct hfs_bnode_desc, num_recs), node->num_recs);
>   
> -	if (rec_off == end_off)
> -		goto skip;
>   	size = fd->keylength + fd->entrylength;
>   
> +	if (rec_off == end_off) {
> +		src = fd->keyoffset;
> +		hfs_bnode_clear(node, src, size);
> +		goto skip;
> +	}
> +
>   	do {
>   		data_off = hfs_bnode_read_u16(node, rec_off);
>   		hfs_bnode_write_u16(node, rec_off + 2, data_off - size);
> @@ -219,9 +224,23 @@ int hfs_brec_remove(struct hfs_find_data *fd)
>   	} while (rec_off >= end_off);
>   
>   	/* fill hole */
> -	hfs_bnode_move(node, fd->keyoffset, fd->keyoffset + size,
> -		       data_off - fd->keyoffset - size);
> +	dst = fd->keyoffset;
> +	src = fd->keyoffset + size;
> +	len = data_off - src;
> +
> +	hfs_bnode_move(node, dst, src, len);
> +
> +	src = dst + len;
> +	len = data_off - src;
> +
> +	hfs_bnode_clear(node, src, len);
> +
>   skip:
> +	/*
> +	 * Remove the obsolete offset to free space.
> +	 */
> +	hfs_bnode_write_u16(node, end_off, 0);
> +
>   	hfs_bnode_dump(node);
>   	if (!fd->record)
>   		hfs_brec_update_parent(fd);

Thx,
Yangtao

Re: [PATCH] hfs: clear offset and space out of valid records in b-tree node

[Viacheslav Dubeyko]

On Wed, 2025-09-03 at 16:04 +0800, Yangtao Li wrote:
> Hi Slava,
> 
> 在 2025/8/16 03:49, Viacheslav Dubeyko 写道:
> > Currently, hfs_brec_remove() executes moving records
> > towards the location of deleted record and it updates
> > offsets of moved records. However, the hfs_brec_remove()
> > logic ignores the "mess" of b-tree node's free space and
> > it doesn't touch the offsets out of records number.
> > Potentially, it could confuse fsck or driver logic or
> > to be a reason of potential corruption cases.
> 
> Patch looks good, and I don't object to it.
> But I don't know what dose it mean
> 
> 'it could confuse fsck or driver logic or to be a reason of potential
> corruption cases.'
> 
> 
> What cases?
> > 

The idea is simple. Let's imagine that we still keep the offsets of
deleted records and the "mess" inside of the node after records
movement. Currently, only number of records in node's header protects
from accessing the offsets of deleted records. But if records number
field in node's header will be corrupted somehow, then offsets of
deleted records will be considered like the valid offsets. As a result,
logic of driver or FSCK will try to access the "records" in the node's
body. And this logic will operate with the "mess" or "garbage" that we
have after records movements. Finally, it could result in pretty not
trivial issues.

Thanks,
Slava.