Re: [PATCH 2/2] btrfs: refresh dir last index during a rewinddir(3) call

[Filipe Manana <fdmanana@kernel.org>]

On Sat, Sep 09, 2023 at 01:08:32PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When opening a directory we find what's the index of its last entry and
> then store it in the directory's file handle private data (struct
> btrfs_file_private::last_index), so that in the case new directory entries
> are added to a directory after an opendir(3) call we don't end up in an
> infinite loop (see commit 9b378f6ad48c ("btrfs: fix infinite directory
> reads")) when calling readdir(3).
> 
> However once rewinddir(3) is called, POSIX states [1] that any new
> directory entries added after the previous opendir(3) call, must be
> returned by subsequent calls to readdir(3):
> 
>   "The rewinddir() function shall reset the position of the directory
>    stream to which dirp refers to the beginning of the directory.
>    It shall also cause the directory stream to refer to the current
>    state of the corresponding directory, as a call to opendir() would
>    have done."
> 
> We currently don't refresh the last_index field of the struct
> btrfs_file_private associated to the directory, so after a rewinddir(3)
> we are not returning any new entries added after the opendir(3) call.
> 
> Fix this by finding the current last index of the directory when llseek
> is called agains the directory.
> 
> This can be reproduced by the following C program provided by Ian Johnson:
> 
>    #include <dirent.h>
>    #include <stdio.h>
> 
>    int main(void) {
>      DIR *dir = opendir("test");
> 
>      FILE *file;
>      file = fopen("test/1", "w");
>      fwrite("1", 1, 1, file);
>      fclose(file);
> 
>      file = fopen("test/2", "w");
>      fwrite("2", 1, 1, file);
>      fclose(file);
> 
>      rewinddir(dir);
> 
>      struct dirent *entry;
>      while ((entry = readdir(dir))) {
>         printf("%s\n", entry->d_name);
>      }
>      closedir(dir);
>      return 0;
>    }

Missing the reference [1] here:

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/rewinddir.html

> 
> Reported-by: Ian Johnson <ian@ianjohnson.dev>

Now also (as per the reply in the linked thread):

Tested-by: Ian Johnson <ian@ianjohnson.dev>

> Link: https://lore.kernel.org/linux-btrfs/YR1P0S.NGASEG570GJ8@ianjohnson.dev/
> Fixes: 9b378f6ad48c ("btrfs: fix infinite directory reads")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  fs/btrfs/inode.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index df035211bdf0..006ca4cb4788 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -5820,6 +5820,19 @@ static int btrfs_opendir(struct inode *inode, struct file *file)
>  	return 0;
>  }
>  
> +static loff_t btrfs_dir_llseek(struct file *file, loff_t offset, int whence)
> +{
> +	struct btrfs_file_private *private = file->private_data;
> +	int ret;
> +
> +	ret = btrfs_get_dir_last_index(BTRFS_I(file_inode(file)),
> +				       &private->last_index);
> +	if (ret)
> +		return ret;
> +
> +	return generic_file_llseek(file, offset, whence);
> +}
> +
>  struct dir_entry {
>  	u64 ino;
>  	u64 offset;
> @@ -10893,7 +10906,7 @@ static const struct inode_operations btrfs_dir_inode_operations = {
>  };
>  
>  static const struct file_operations btrfs_dir_file_operations = {
> -	.llseek		= generic_file_llseek,
> +	.llseek		= btrfs_dir_llseek,
>  	.read		= generic_read_dir,
>  	.iterate_shared	= btrfs_real_readdir,
>  	.open		= btrfs_opendir,
> -- 
> 2.40.1
>