On Tue 03-09-24 13:34:30, Christian Brauner wrote:
> On Fri, Aug 30, 2024 at 03:04:55PM GMT, Christian Brauner wrote:
> > Store the cookie to detect concurrent seeks on directories in
> > file->private_data.
> >
> > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> > ---
> > fs/proc/base.c | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index 72a1acd03675..8a8aab6b9801 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -3870,12 +3870,12 @@ static int proc_task_readdir(struct file *file, struct dir_context *ctx)
> > if (!dir_emit_dots(file, ctx))
> > return 0;
> >
> > - /* f_version caches the tgid value that the last readdir call couldn't
> > - * return. lseek aka telldir automagically resets f_version to 0.
> > + /* We cache the tgid value that the last readdir call couldn't
> > + * return and lseek resets it to 0.
> > */
> > ns = proc_pid_ns(inode->i_sb);
> > - tid = (int)file->f_version;
> > - file->f_version = 0;
> > + tid = (int)(intptr_t)file->private_data;
> > + file->private_data = NULL;
> > for (task = first_tid(proc_pid(inode), tid, ctx->pos - 2, ns);
> > task;
> > task = next_tid(task), ctx->pos++) {
> > @@ -3890,7 +3890,7 @@ static int proc_task_readdir(struct file *file, struct dir_context *ctx)
> > proc_task_instantiate, task, NULL)) {
> > /* returning this tgid failed, save it as the first
> > * pid for the next readir call */
> > - file->f_version = (u64)tid;
> > + file->private_data = (void *)(intptr_t)tid;
> > put_task_struct(task);
> > break;
> > }
> > @@ -3915,6 +3915,12 @@ static int proc_task_getattr(struct mnt_idmap *idmap,
> > return 0;
> > }
> >
> > +static loff_t proc_dir_llseek(struct file *file, loff_t offset, int whence)
> > +{
> > + return generic_llseek_cookie(file, offset, whence,
> > + (u64 *)(uintptr_t)&file->private_data);
>
> Btw, this is fixed in-tree (I did send out an unfixed version):
>
> static loff_t proc_dir_llseek(struct file *file, loff_t offset, int whence)
> {
> u64 cookie = 1;
> loff_t off;
>
> off = generic_llseek_cookie(file, offset, whence, &cookie);
> if (!cookie)
> file->private_data = NULL; /* serialized by f_pos_lock */
> return off;
> }
Ah, midair collision :). This looks better just why don't you store the
cookie unconditionally in file->private_data? This way proc_dir_llseek()
makes assumptions about how generic_llseek_cookie() uses the cookie which
unnecessarily spreads internal VFS knowledge into filesystems...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
