On Fri 30-08-24 15:04:58, Christian Brauner wrote:
> Store the cookie to detect concurrent seeks on directories in
> file->private_data.
>
> Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/ubifs/dir.c | 65 ++++++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 47 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index c77ea57fe696..76bcafa92200 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -555,6 +555,11 @@ static unsigned int vfs_dent_type(uint8_t type)
> return 0;
> }
>
> +struct ubifs_dir_data {
> + struct ubifs_dent_node *dent;
> + u64 cookie;
> +};
> +
> /*
> * The classical Unix view for directory is that it is a linear array of
> * (name, inode number) entries. Linux/VFS assumes this model as well.
> @@ -582,6 +587,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
> struct inode *dir = file_inode(file);
> struct ubifs_info *c = dir->i_sb->s_fs_info;
> bool encrypted = IS_ENCRYPTED(dir);
> + struct ubifs_dir_data *data = file->private_data;
>
> dbg_gen("dir ino %lu, f_pos %#llx", dir->i_ino, ctx->pos);
>
> @@ -604,27 +610,27 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
> fstr_real_len = fstr.len;
> }
>
> - if (file->f_version == 0) {
> + if (data->cookie == 0) {
> /*
> - * The file was seek'ed, which means that @file->private_data
> + * The file was seek'ed, which means that @data->dent
> * is now invalid. This may also be just the first
> * 'ubifs_readdir()' invocation, in which case
> - * @file->private_data is NULL, and the below code is
> + * @data->dent is NULL, and the below code is
> * basically a no-op.
> */
> - kfree(file->private_data);
> - file->private_data = NULL;
> + kfree(data->dent);
> + data->dent = NULL;
> }
>
> /*
> - * 'generic_file_llseek()' unconditionally sets @file->f_version to
> - * zero, and we use this for detecting whether the file was seek'ed.
> + * 'ubifs_dir_llseek()' sets @data->cookie to zero, and we use this
> + * for detecting whether the file was seek'ed.
> */
> - file->f_version = 1;
> + data->cookie = 1;
>
> /* File positions 0 and 1 correspond to "." and ".." */
> if (ctx->pos < 2) {
> - ubifs_assert(c, !file->private_data);
> + ubifs_assert(c, !data->dent);
> if (!dir_emit_dots(file, ctx)) {
> if (encrypted)
> fscrypt_fname_free_buffer(&fstr);
> @@ -641,10 +647,10 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
> }
>
> ctx->pos = key_hash_flash(c, &dent->key);
> - file->private_data = dent;
> + data->dent = dent;
> }
>
> - dent = file->private_data;
> + dent = data->dent;
> if (!dent) {
> /*
> * The directory was seek'ed to and is now readdir'ed.
> @@ -658,7 +664,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
> goto out;
> }
> ctx->pos = key_hash_flash(c, &dent->key);
> - file->private_data = dent;
> + data->dent = dent;
> }
>
> while (1) {
> @@ -701,15 +707,15 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
> goto out;
> }
>
> - kfree(file->private_data);
> + kfree(data->dent);
> ctx->pos = key_hash_flash(c, &dent->key);
> - file->private_data = dent;
> + data->dent = dent;
> cond_resched();
> }
>
> out:
> - kfree(file->private_data);
> - file->private_data = NULL;
> + kfree(data->dent);
> + data->dent = NULL;
>
> if (encrypted)
> fscrypt_fname_free_buffer(&fstr);
> @@ -733,7 +739,10 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
> /* Free saved readdir() state when the directory is closed */
> static int ubifs_dir_release(struct inode *dir, struct file *file)
> {
> - kfree(file->private_data);
> + struct ubifs_dir_data *data = file->private_data;
> +
> + kfree(data->dent);
> + kfree(data);
> file->private_data = NULL;
> return 0;
> }
> @@ -1712,6 +1721,24 @@ int ubifs_getattr(struct mnt_idmap *idmap, const struct path *path,
> return 0;
> }
>
> +static int ubifs_dir_open(struct inode *inode, struct file *file)
> +{
> + struct ubifs_dir_data *data;
> +
> + data = kzalloc(sizeof(struct ubifs_dir_data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> + file->private_data = data;
> + return 0;
> +}
> +
> +static loff_t ubifs_dir_llseek(struct file *file, loff_t offset, int whence)
> +{
> + struct ubifs_dir_data *data = file->private_data;
> +
> + return generic_llseek_cookie(file, offset, whence, &data->cookie);
> +}
> +
> const struct inode_operations ubifs_dir_inode_operations = {
> .lookup = ubifs_lookup,
> .create = ubifs_create,
> @@ -1732,7 +1759,9 @@ const struct inode_operations ubifs_dir_inode_operations = {
> };
>
> const struct file_operations ubifs_dir_operations = {
> - .llseek = generic_file_llseek,
> + .open = ubifs_dir_open,
> + .release = ubifs_dir_release,
> + .llseek = ubifs_dir_llseek,
> .release = ubifs_dir_release,
> .read = generic_read_dir,
> .iterate_shared = ubifs_readdir,
>
> --
> 2.45.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
