[PATCH 0/2] hfs, hfsplus: do not end readdir early after an unlink

[PATCH 0/2] hfs, hfsplus: do not end readdir early after an unlink

[Hengyu Liang]

Since the readdir rework in v7.2, HFS and HFS+ end a directory walk
early when the caller deletes the entries it has read. Both filesystems
got the same change, so the two patches make the same fix.

Tested on hfs for-next with KASAN and lockdep. The reproducers pass and
generic/637 still passes on both filesystems. A run of 30 generic tests
gives the same results with and without the patches on HFS+. On HFS,
fsck finds the test volume inconsistent also without the patches, so I
ran generic/013, 028 and 637 on fresh volumes six times each: 013 fails
and the other two pass, with and without the patches.

Hengyu Liang (2):
  hfs: do not end readdir at i_size after an unlink
  hfsplus: do not end readdir at i_size after an unlink

 fs/hfs/dir.c     | 27 +++++++++++++++++++--------
 fs/hfsplus/dir.c | 27 +++++++++++++++++++--------
 2 files changed, 38 insertions(+), 16 deletions(-)


base-commit: 34ee5f8c2198db9585da6c181c50869d2eb43c3a
-- 
2.53.0

[PATCH 1/2] hfs: do not end readdir at i_size after an unlink

[Hengyu Liang]

Commit 7fde7e806657 ("hfs: rework hfsplus_readdir() logic") stopped
moving the position of open directories back when an entry is deleted.

However, hfs_readdir() still ends when ctx->pos reaches i_size, and
i_size drops on every delete. As of now, a program that reads a
directory and deletes what it read sees the directory end early and
leaves files behind.

The issue can be reproduced with BusyBox rm on a new hfs volume:

    mkdir /mnt/d
    (cd /mnt/d && for i in $(seq 1500); do : > f$i; done)
    rm -r /mnt/d

Before commit 7fde7e806657 ("hfs: rework hfsplus_readdir() logic"),
the directory is removed. After that commit, the result is:

    rm: can't remove '/mnt/d': Directory not empty

and 304 files are left.

This patch will end a walk that continues from the saved key where the
records of the directory end, and keep the i_size check for a walk that
starts from a position.

Fixes: 7fde7e806657 ("hfs: rework hfsplus_readdir() logic")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
Application impact (Ubuntu 26.04 user space): a folder with 1500 files
on an HFS volume is moved to the trash, then the trash is emptied with
"gio trash --empty" (the call behind "Empty Trash" in GNOME Files, done
by gvfsd-trash 1.60). On v7.1, all files are removed. On v7.2, the trash
looks empty, but 478 files stay in .Trash-0/expunged and their space is
not freed.

 fs/hfs/dir.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
index b6b8945712a9..b0858512f4d2 100644
--- a/fs/hfs/dir.c
+++ b/fs/hfs/dir.c
@@ -51,10 +51,16 @@ static int hfs_readdir(struct file *file, struct dir_context *ctx)
 	char strbuf[HFS_MAX_NAMELEN];
 	union hfs_cat_rec entry;
 	struct hfs_find_data fd;
-	struct hfs_readdir_data *rd;
+	struct hfs_readdir_data *rd = file->private_data;
+	/*
+	 * i_size follows the valence and drops on unlink, so it says nothing
+	 * about a walk that continues from a saved key. That walk ends where
+	 * the records of this directory end.
+	 */
+	bool resume = rd && rd->pos == ctx->pos;
 	u16 type;
 
-	if (ctx->pos >= inode->i_size)
+	if (!resume && ctx->pos >= inode->i_size)
 		return 0;
 
 	err = hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
@@ -93,10 +99,9 @@ static int hfs_readdir(struct file *file, struct dir_context *ctx)
 			goto out;
 		ctx->pos = 2;
 	}
-	if (ctx->pos >= inode->i_size)
+	if (!resume && ctx->pos >= inode->i_size)
 		goto out;
-	rd = file->private_data;
-	if (rd && rd->pos == ctx->pos) {
+	if (resume) {
 		memcpy(fd.search_key, &rd->key, sizeof(struct hfs_cat_key));
 		err = hfs_brec_find(&fd);
 		if (err == -ENOENT)
@@ -105,10 +110,12 @@ static int hfs_readdir(struct file *file, struct dir_context *ctx)
 		err = hfs_brec_goto(&fd, ctx->pos - 1);
 	}
 	if (err)
-		goto out;
+		goto end_walk;
 
 	for (;;) {
 		if (be32_to_cpu(fd.key->cat.ParID) != inode->i_ino) {
+			if (resume)
+				goto out;
 			pr_err("walked past end of dir\n");
 			err = -EIO;
 			goto out;
@@ -146,11 +153,11 @@ static int hfs_readdir(struct file *file, struct dir_context *ctx)
 			goto out;
 		}
 		ctx->pos++;
-		if (ctx->pos >= inode->i_size)
+		if (!resume && ctx->pos >= inode->i_size)
 			goto out;
 		err = hfs_brec_goto(&fd, 1);
 		if (err)
-			goto out;
+			goto end_walk;
 	}
 	if (!rd) {
 		rd = kmalloc_obj(struct hfs_readdir_data);
@@ -162,6 +169,10 @@ static int hfs_readdir(struct file *file, struct dir_context *ctx)
 	}
 	rd->pos = ctx->pos;
 	memcpy(&rd->key, &fd.key->cat, sizeof(struct hfs_cat_key));
+end_walk:
+	/* This directory can hold the last records of the catalog. */
+	if (resume && err == -ENOENT)
+		err = 0;
 out:
 	hfs_find_exit(&fd);
 	return err;
-- 
2.53.0

Re: [PATCH 1/2] hfs: do not end readdir at i_size after an unlink

[Viacheslav Dubeyko]

On Thu, 2026-11-12 at 06:17 -0500, Hengyu Liang wrote:
> Commit 7fde7e806657 ("hfs: rework hfsplus_readdir() logic") stopped
> moving the position of open directories back when an entry is
> deleted.
> 
> However, hfs_readdir() still ends when ctx->pos reaches i_size, and
> i_size drops on every delete. As of now, a program that reads a
> directory and deletes what it read sees the directory end early and
> leaves files behind.
> 
> The issue can be reproduced with BusyBox rm on a new hfs volume:
> 
>     mkdir /mnt/d
>     (cd /mnt/d && for i in $(seq 1500); do : > f$i; done)
>     rm -r /mnt/d
> 
> Before commit 7fde7e806657 ("hfs: rework hfsplus_readdir() logic"),
> the directory is removed. After that commit, the result is:
> 
>     rm: can't remove '/mnt/d': Directory not empty
> 
> and 304 files are left.
> 

Currently, I don't understand how the issue happens. Could you please
explain the issue at first?

Thanks,
Slava.

> This patch will end a walk that continues from the saved key where
> the
> records of the directory end, and keep the i_size check for a walk
> that
> starts from a position.
> 
> Fixes: 7fde7e806657 ("hfs: rework hfsplus_readdir() logic")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
> ---
> Application impact (Ubuntu 26.04 user space): a folder with 1500
> files
> on an HFS volume is moved to the trash, then the trash is emptied
> with
> "gio trash --empty" (the call behind "Empty Trash" in GNOME Files,
> done
> by gvfsd-trash 1.60). On v7.1, all files are removed. On v7.2, the
> trash
> looks empty, but 478 files stay in .Trash-0/expunged and their space
> is
> not freed.
> 
>  fs/hfs/dir.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
> index b6b8945712a9..b0858512f4d2 100644
> --- a/fs/hfs/dir.c
> +++ b/fs/hfs/dir.c
> @@ -51,10 +51,16 @@ static int hfs_readdir(struct file *file, struct
> dir_context *ctx)
>  	char strbuf[HFS_MAX_NAMELEN];
>  	union hfs_cat_rec entry;
>  	struct hfs_find_data fd;
> -	struct hfs_readdir_data *rd;
> +	struct hfs_readdir_data *rd = file->private_data;
> +	/*
> +	 * i_size follows the valence and drops on unlink, so it
> says nothing
> +	 * about a walk that continues from a saved key. That walk
> ends where
> +	 * the records of this directory end.
> +	 */
> +	bool resume = rd && rd->pos == ctx->pos;
>  	u16 type;
>  
> -	if (ctx->pos >= inode->i_size)
> +	if (!resume && ctx->pos >= inode->i_size)
>  		return 0;
>  
>  	err = hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
> @@ -93,10 +99,9 @@ static int hfs_readdir(struct file *file, struct
> dir_context *ctx)
>  			goto out;
>  		ctx->pos = 2;
>  	}
> -	if (ctx->pos >= inode->i_size)
> +	if (!resume && ctx->pos >= inode->i_size)
>  		goto out;
> -	rd = file->private_data;
> -	if (rd && rd->pos == ctx->pos) {
> +	if (resume) {
>  		memcpy(fd.search_key, &rd->key, sizeof(struct
> hfs_cat_key));
>  		err = hfs_brec_find(&fd);
>  		if (err == -ENOENT)
> @@ -105,10 +110,12 @@ static int hfs_readdir(struct file *file,
> struct dir_context *ctx)
>  		err = hfs_brec_goto(&fd, ctx->pos - 1);
>  	}
>  	if (err)
> -		goto out;
> +		goto end_walk;
>  
>  	for (;;) {
>  		if (be32_to_cpu(fd.key->cat.ParID) != inode->i_ino)
> {
> +			if (resume)
> +				goto out;
>  			pr_err("walked past end of dir\n");
>  			err = -EIO;
>  			goto out;
> @@ -146,11 +153,11 @@ static int hfs_readdir(struct file *file,
> struct dir_context *ctx)
>  			goto out;
>  		}
>  		ctx->pos++;
> -		if (ctx->pos >= inode->i_size)
> +		if (!resume && ctx->pos >= inode->i_size)
>  			goto out;
>  		err = hfs_brec_goto(&fd, 1);
>  		if (err)
> -			goto out;
> +			goto end_walk;
>  	}
>  	if (!rd) {
>  		rd = kmalloc_obj(struct hfs_readdir_data);
> @@ -162,6 +169,10 @@ static int hfs_readdir(struct file *file, struct
> dir_context *ctx)
>  	}
>  	rd->pos = ctx->pos;
>  	memcpy(&rd->key, &fd.key->cat, sizeof(struct hfs_cat_key));
> +end_walk:
> +	/* This directory can hold the last records of the catalog.
> */
> +	if (resume && err == -ENOENT)
> +		err = 0;
>  out:
>  	hfs_find_exit(&fd);
>  	return err;

[PATCH 2/2] hfsplus: do not end readdir at i_size after an unlink

[Hengyu Liang]

Commit 4b0496432844 ("hfsplus: rework hfsplus_readdir() logic") stopped
moving the position of open directories back when an entry is deleted.

However, hfsplus_readdir() still ends when ctx->pos reaches i_size, and
i_size drops on every delete. As of now, a program that reads a
directory and deletes what it read sees the directory end early and
leaves files behind.

The issue can be reproduced with BusyBox rm on a new hfsplus volume:

    mkdir /mnt/d
    (cd /mnt/d && for i in $(seq 1500); do : > f$i; done)
    rm -r /mnt/d

Before commit 4b0496432844 ("hfsplus: rework hfsplus_readdir() logic"),
the directory is removed. After that commit, the result is:

    rm: can't remove '/mnt/d': Directory not empty

and 304 files are left.

This patch will end a walk that continues from the saved key where the
records of the directory end, and keep the i_size check for a walk that
starts from a position.

Fixes: 4b0496432844 ("hfsplus: rework hfsplus_readdir() logic")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
Application impact (Ubuntu 26.04 user space): a folder with 1500 files
on an HFS+ volume is moved to the trash, then the trash is emptied with
"gio trash --empty" (the call behind "Empty Trash" in GNOME Files, done
by gvfsd-trash 1.60). On v7.1, all files are removed. On v7.2, the trash
looks empty, but 478 files stay in .Trash-0/expunged and their space is
not freed.

 fs/hfsplus/dir.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index b3a1193a491f..81fe8a3e07c9 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -135,10 +135,16 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
 	char *strbuf;
 	hfsplus_cat_entry entry;
 	struct hfs_find_data fd;
-	struct hfsplus_readdir_data *rd;
+	struct hfsplus_readdir_data *rd = file->private_data;
+	/*
+	 * i_size follows the valence and drops on unlink, so it says nothing
+	 * about a walk that continues from a saved key. That walk ends where
+	 * the records of this directory end.
+	 */
+	bool resume = rd && rd->pos == file->f_pos;
 	u16 type;
 
-	if (file->f_pos >= inode->i_size)
+	if (!resume && file->f_pos >= inode->i_size)
 		return 0;
 
 	err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
@@ -183,10 +189,9 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
 			goto out;
 		ctx->pos = 2;
 	}
-	if (ctx->pos >= inode->i_size)
+	if (!resume && ctx->pos >= inode->i_size)
 		goto out;
-	rd = file->private_data;
-	if (rd && rd->pos == ctx->pos) {
+	if (resume) {
 		memcpy(fd.search_key, &rd->key, sizeof(struct hfsplus_cat_key));
 		err = hfs_brec_find(&fd, hfs_find_rec_by_key);
 		if (err == -ENOENT)
@@ -195,9 +200,11 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
 		err = hfs_brec_goto(&fd, ctx->pos - 1);
 	}
 	if (err)
-		goto out;
+		goto end_walk;
 	for (;;) {
 		if (be32_to_cpu(fd.key->cat.parent) != inode->i_ino) {
+			if (resume)
+				goto out;
 			pr_err("walked past end of dir\n");
 			err = -EIO;
 			goto out;
@@ -263,11 +270,11 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
 		}
 next:
 		ctx->pos++;
-		if (ctx->pos >= inode->i_size)
+		if (!resume && ctx->pos >= inode->i_size)
 			goto out;
 		err = hfs_brec_goto(&fd, 1);
 		if (err)
-			goto out;
+			goto end_walk;
 	}
 	if (!rd) {
 		rd = kmalloc_obj(struct hfsplus_readdir_data);
@@ -279,6 +286,10 @@ static int hfsplus_readdir(struct file *file, struct dir_context *ctx)
 	}
 	rd->pos = ctx->pos;
 	memcpy(&rd->key, fd.key, sizeof(struct hfsplus_cat_key));
+end_walk:
+	/* This directory can hold the last records of the catalog. */
+	if (resume && err == -ENOENT)
+		err = 0;
 out:
 	kfree(strbuf);
 	hfs_find_exit(&fd);
-- 
2.53.0

Re: [PATCH 2/2] hfsplus: do not end readdir at i_size after an unlink

[Viacheslav Dubeyko]

On Thu, 2026-11-12 at 06:17 -0500, Hengyu Liang wrote:
> Commit 4b0496432844 ("hfsplus: rework hfsplus_readdir() logic")
> stopped
> moving the position of open directories back when an entry is
> deleted.
> 
> However, hfsplus_readdir() still ends when ctx->pos reaches i_size,
> and
> i_size drops on every delete. As of now, a program that reads a
> directory and deletes what it read sees the directory end early and
> leaves files behind.
> 
> The issue can be reproduced with BusyBox rm on a new hfsplus volume:
> 
>     mkdir /mnt/d
>     (cd /mnt/d && for i in $(seq 1500); do : > f$i; done)
>     rm -r /mnt/d

Frankly speaking, I don't quite follow what is the issue of the fix. Do
you mean that we create and delete folder in one thread and execute the
readdir in another one? Or do you mean that application has several
threads and one thread do readdir but another one delete file by file?
How this deletion is executed? I mean which files are deleted?

Even if we delete files in folder then i_size is dropping gradually. If
we traversing the folder's content and ctx->pos >= inode->i_size, then
it sounds that we already showed the existing content. Do you mean that
if we deleted the range that we already processed, then we miss the
rest of undeleted items?

> 
> Before commit 4b0496432844 ("hfsplus: rework hfsplus_readdir()
> logic"),
> the directory is removed. After that commit, the result is:
> 
>     rm: can't remove '/mnt/d': Directory not empty
> 
> and 304 files are left.
> 
> This patch will end a walk that continues from the saved key where
> the
> records of the directory end, and keep the i_size check for a walk
> that
> starts from a position.
> 
> Fixes: 4b0496432844 ("hfsplus: rework hfsplus_readdir() logic")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
> ---
> Application impact (Ubuntu 26.04 user space): a folder with 1500
> files
> on an HFS+ volume is moved to the trash, then the trash is emptied
> with
> "gio trash --empty" (the call behind "Empty Trash" in GNOME Files,
> done
> by gvfsd-trash 1.60). On v7.1, all files are removed. On v7.2, the
> trash
> looks empty, but 478 files stay in .Trash-0/expunged and their space
> is
> not freed.
> 
>  fs/hfsplus/dir.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
> index b3a1193a491f..81fe8a3e07c9 100644
> --- a/fs/hfsplus/dir.c
> +++ b/fs/hfsplus/dir.c
> @@ -135,10 +135,16 @@ static int hfsplus_readdir(struct file *file,
> struct dir_context *ctx)
>  	char *strbuf;
>  	hfsplus_cat_entry entry;
>  	struct hfs_find_data fd;
> -	struct hfsplus_readdir_data *rd;
> +	struct hfsplus_readdir_data *rd = file->private_data;
> +	/*
> +	 * i_size follows the valence and drops on unlink, so it
> says nothing
> +	 * about a walk that continues from a saved key. That walk
> ends where
> +	 * the records of this directory end.
> +	 */

I really dislike the comment in the variable declaration section.

> +	bool resume = rd && rd->pos == file->f_pos;

Is it possible that rd is NULL here?

I don't really follow how the resume variable can help at all.
Probably, I am missing the case. Could you please explain the issue at
first?

Thanks,
Slava.

>  	u16 type;
>  
> -	if (file->f_pos >= inode->i_size)
> +	if (!resume && file->f_pos >= inode->i_size)
>  		return 0;
>  
>  	err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
> @@ -183,10 +189,9 @@ static int hfsplus_readdir(struct file *file,
> struct dir_context *ctx)
>  			goto out;
>  		ctx->pos = 2;
>  	}
> -	if (ctx->pos >= inode->i_size)
> +	if (!resume && ctx->pos >= inode->i_size)
>  		goto out;
> -	rd = file->private_data;
> -	if (rd && rd->pos == ctx->pos) {
> +	if (resume) {
>  		memcpy(fd.search_key, &rd->key, sizeof(struct
> hfsplus_cat_key));
>  		err = hfs_brec_find(&fd, hfs_find_rec_by_key);
>  		if (err == -ENOENT)
> @@ -195,9 +200,11 @@ static int hfsplus_readdir(struct file *file,
> struct dir_context *ctx)
>  		err = hfs_brec_goto(&fd, ctx->pos - 1);
>  	}
>  	if (err)
> -		goto out;
> +		goto end_walk;
>  	for (;;) {
>  		if (be32_to_cpu(fd.key->cat.parent) != inode->i_ino)
> {
> +			if (resume)
> +				goto out;
>  			pr_err("walked past end of dir\n");
>  			err = -EIO;
>  			goto out;
> @@ -263,11 +270,11 @@ static int hfsplus_readdir(struct file *file,
> struct dir_context *ctx)
>  		}
>  next:
>  		ctx->pos++;
> -		if (ctx->pos >= inode->i_size)
> +		if (!resume && ctx->pos >= inode->i_size)
>  			goto out;
>  		err = hfs_brec_goto(&fd, 1);
>  		if (err)
> -			goto out;
> +			goto end_walk;
>  	}
>  	if (!rd) {
>  		rd = kmalloc_obj(struct hfsplus_readdir_data);
> @@ -279,6 +286,10 @@ static int hfsplus_readdir(struct file *file,
> struct dir_context *ctx)
>  	}
>  	rd->pos = ctx->pos;
>  	memcpy(&rd->key, fd.key, sizeof(struct hfsplus_cat_key));
> +end_walk:
> +	/* This directory can hold the last records of the catalog.
> */
> +	if (resume && err == -ENOENT)
> +		err = 0;
>  out:
>  	kfree(strbuf);
>  	hfs_find_exit(&fd);