diff mbox series

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

Message ID f2ce20268ec99d4d4e1392a200d75309a0b41acc.1694260751.git.fdmanana@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: updates for directory reading | expand

Commit Message

Filipe Manana Sept. 9, 2023, 12:08 p.m. UTC 
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;
   }

Reported-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(-)

Comments

Filipe Manana Sept. 11, 2023, 10:36 a.m. UTC | #1 
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
>
diff mbox series

Patch

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,