Re: [PATCH] cifs: don't leak -EAGAIN for stat() during reconnect

[Pavel Shilovsky <piastryyy@gmail.com>]

вт, 18 февр. 2020 г. в 15:27, Steve French <smfrench@gmail.com>:
>
> merged into cifs-2.6.git for-next
>
> On Tue, Feb 18, 2020 at 2:07 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> >
> > If from cifs_revalidate_dentry_attr() the SMB2/QUERY_INFO call fails with an
> > error, such as STATUS_SESSION_EXPIRED, causing the session to be reconnected
> > it is possible we will leak -EAGAIN back to the application even for
> > system calls such as stat() where this is not a valid error.
> >
> > Fix this by re-trying the operation from within cifs_revalidate_dentry_attr()
> > if cifs_get_inode_info*() returns -EAGAIN.
> >
> > This fixes stat() and possibly also other system calls that uses
> > cifs_revalidate_dentry*().
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> > ---
> >  fs/cifs/inode.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> > index b5e6635c578e..1212ace05258 100644
> > --- a/fs/cifs/inode.c
> > +++ b/fs/cifs/inode.c
> > @@ -2073,6 +2073,7 @@ int cifs_revalidate_dentry_attr(struct dentry *dentry)
> >         struct inode *inode = d_inode(dentry);
> >         struct super_block *sb = dentry->d_sb;
> >         char *full_path = NULL;
> > +       int count = 0;
> >
> >         if (inode == NULL)
> >                 return -ENOENT;
> > @@ -2094,15 +2095,18 @@ int cifs_revalidate_dentry_attr(struct dentry *dentry)
> >                  full_path, inode, inode->i_count.counter,
> >                  dentry, cifs_get_time(dentry), jiffies);
> >
> > +again:
> >         if (cifs_sb_master_tcon(CIFS_SB(sb))->unix_ext)
> >                 rc = cifs_get_inode_info_unix(&inode, full_path, sb, xid);
> >         else
> >                 rc = cifs_get_inode_info(&inode, full_path, NULL, sb,
> >                                          xid, NULL);
> > -
> > +       if (is_retryable_error(rc) && count++ < 10)
> > +               goto again;

If there is interrupt error, you will end up doing 10 attempts with
the same outcome - interrupt error. Such errors should be returned to
the upper layers to be handled correctly (restart of a system call or
return of EINTR error to the user space).

Please revert to your original version that handles EAGAIN only.

--
Best regards,
Pavel Shilovsky

> >  out:
> >         kfree(full_path);
> >         free_xid(xid);
> > +
> >         return rc;
> >  }
> >
> > --
> > 2.13.6
> >
>
>
> --
> Thanks,
>
> Steve