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@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>
> ---
> 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;