Re: [PATCH v1 1/3] fs: add parameter use_fpos to iterate_dir function

[Pavel Begunkov <asml.silence@gmail.com>]

On 11/23/21 18:10, Stefan Roesch wrote:
> This adds the use_fpos parameter to the iterate_dir function.
> If use_fpos is true it uses the file position in the file
> structure (existing behavior). If use_fpos is false, it uses
> the pos in the context structure.
> 
> This change is required to support getdents in io_uring.
> 
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---
>   fs/exportfs/expfs.c    |  2 +-
>   fs/nfsd/nfs4recover.c  |  2 +-
>   fs/nfsd/vfs.c          |  2 +-
>   fs/overlayfs/readdir.c |  6 +++---
>   fs/readdir.c           | 28 ++++++++++++++++++++--------
>   include/linux/fs.h     |  2 +-
>   6 files changed, 27 insertions(+), 15 deletions(-)
> 
[...]
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 09e8ed7d4161..8ea5b5f45a78 100644
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -21,6 +21,7 @@
>   #include <linux/unistd.h>
>   #include <linux/compat.h>
>   #include <linux/uaccess.h>
> +#include "internal.h"
>   
>   #include <asm/unaligned.h>
>   
> @@ -36,8 +37,14 @@
>   	unsafe_copy_to_user(dst, src, len, label);		\
>   } while (0)
>   
> -
> -int iterate_dir(struct file *file, struct dir_context *ctx)
> +/**
> + * iterate_dir - iterate over directory
> + * @file    : pointer to file struct of directory
> + * @ctx     : pointer to directory ctx structure
> + * @use_fpos: true : use file offset
> + *            false: use pos in ctx structure
> + */
> +int iterate_dir(struct file *file, struct dir_context *ctx, bool use_fpos)
>   {
>   	struct inode *inode = file_inode(file);
>   	bool shared = false;
> @@ -60,12 +67,17 @@ int iterate_dir(struct file *file, struct dir_context *ctx)
>   
>   	res = -ENOENT;
>   	if (!IS_DEADDIR(inode)) {
> -		ctx->pos = file->f_pos;
> +		if (use_fpos)
> +			ctx->pos = file->f_pos;

One more thing I haven't noticed before, should pos be sanitised
somehow if passed from the userspace? Do filesystems handle it
well?


> +
>   		if (shared)
>   			res = file->f_op->iterate_shared(file, ctx);
>   		else
>   			res = file->f_op->iterate(file, ctx);
> -		file->f_pos = ctx->pos;
> +
> +		if (use_fpos)
> +			file->f_pos = ctx->pos;
> +
>   		fsnotify_access(file);
>   		file_accessed(file);
>   	}


-- 
Pavel Begunkov