Re: [PATCH v2 1/2] cifs: improve checking of DFS links over STATUS_OBJECT_NAME_INVALID

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



updated to add RB tag and added:

CC: stable@xxxxxxxxxxxxxxx # 6.2

On Tue, Feb 28, 2023 at 4:21 PM ronnie sahlberg
<ronniesahlberg@xxxxxxxxx> wrote:
>
> Reviewed-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
>
> On Wed, 1 Mar 2023 at 08:15, Paulo Alcantara <pc@xxxxxxxxxxxxx> wrote:
> >
> > Do not map STATUS_OBJECT_NAME_INVALID to -EREMOTE under non-DFS
> > shares, or 'nodfs' mounts or CONFIG_CIFS_DFS_UPCALL=n builds.
> > Otherwise, in the slow path, get a referral to figure out whether it
> > is an actual DFS link.
> >
> > This could be simply reproduced under a non-DFS share by running the
> > following
> >
> >   $ mount.cifs //srv/share /mnt -o ...
> >   $ cat /mnt/$(printf '\U110000')
> >   cat: '/mnt/'$'\364\220\200\200': Object is remote
> >
> > Fixes: c877ce47e137 ("cifs: reduce roundtrips on create/qinfo requests")
> > Signed-off-by: Paulo Alcantara (SUSE) <pc@xxxxxxxxxxxxx>
> > ---
> > v1 -> v2: fixed mem leak on PTR_ERR(ref_path) != -EINVAL
> >
> >  fs/cifs/cifsproto.h | 20 ++++++++++----
> >  fs/cifs/misc.c      | 67 +++++++++++++++++++++++++++++++++++++++++++++
> >  fs/cifs/smb2inode.c | 21 +++++++-------
> >  fs/cifs/smb2ops.c   | 23 +++++++++-------
> >  4 files changed, 106 insertions(+), 25 deletions(-)
> >
> > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> > index b7a36ebd0f2f..20a2f0f3f682 100644
> > --- a/fs/cifs/cifsproto.h
> > +++ b/fs/cifs/cifsproto.h
> > @@ -667,11 +667,21 @@ static inline int get_dfs_path(const unsigned int xid, struct cifs_ses *ses,
> >  int match_target_ip(struct TCP_Server_Info *server,
> >                     const char *share, size_t share_len,
> >                     bool *result);
> > -
> > -int cifs_dfs_query_info_nonascii_quirk(const unsigned int xid,
> > -                                      struct cifs_tcon *tcon,
> > -                                      struct cifs_sb_info *cifs_sb,
> > -                                      const char *dfs_link_path);
> > +int cifs_inval_name_dfs_link_error(const unsigned int xid,
> > +                                  struct cifs_tcon *tcon,
> > +                                  struct cifs_sb_info *cifs_sb,
> > +                                  const char *full_path,
> > +                                  bool *islink);
> > +#else
> > +static inline int cifs_inval_name_dfs_link_error(const unsigned int xid,
> > +                                  struct cifs_tcon *tcon,
> > +                                  struct cifs_sb_info *cifs_sb,
> > +                                  const char *full_path,
> > +                                  bool *islink)
> > +{
> > +       *islink = false;
> > +       return 0;
> > +}
> >  #endif
> >
> >  static inline int cifs_create_options(struct cifs_sb_info *cifs_sb, int options)
> > diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> > index 2905734eb289..0c6c1fc8dae9 100644
> > --- a/fs/cifs/misc.c
> > +++ b/fs/cifs/misc.c
> > @@ -21,6 +21,7 @@
> >  #include "cifsfs.h"
> >  #ifdef CONFIG_CIFS_DFS_UPCALL
> >  #include "dns_resolve.h"
> > +#include "dfs_cache.h"
> >  #endif
> >  #include "fs_context.h"
> >  #include "cached_dir.h"
> > @@ -1198,4 +1199,70 @@ int cifs_update_super_prepath(struct cifs_sb_info *cifs_sb, char *prefix)
> >         cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_USE_PREFIX_PATH;
> >         return 0;
> >  }
> > +
> > +/*
> > + * Handle weird Windows SMB server behaviour. It responds with
> > + * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request for
> > + * "\<server>\<dfsname>\<linkpath>" DFS reference, where <dfsname> contains
> > + * non-ASCII unicode symbols.
> > + */
> > +int cifs_inval_name_dfs_link_error(const unsigned int xid,
> > +                                  struct cifs_tcon *tcon,
> > +                                  struct cifs_sb_info *cifs_sb,
> > +                                  const char *full_path,
> > +                                  bool *islink)
> > +{
> > +       struct cifs_ses *ses = tcon->ses;
> > +       size_t len;
> > +       char *path;
> > +       char *ref_path;
> > +
> > +       *islink = false;
> > +
> > +       /*
> > +        * Fast path - skip check when @full_path doesn't have a prefix path to
> > +        * look up or tcon is not DFS.
> > +        */
> > +       if (strlen(full_path) < 2 || !cifs_sb ||
> > +           (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS) ||
> > +           !is_tcon_dfs(tcon) || !ses->server->origin_fullpath)
> > +               return 0;
> > +
> > +       /*
> > +        * Slow path - tcon is DFS and @full_path has prefix path, so attempt
> > +        * to get a referral to figure out whether it is an DFS link.
> > +        */
> > +       len = strnlen(tcon->tree_name, MAX_TREE_SIZE + 1) + strlen(full_path) + 1;
> > +       path = kmalloc(len, GFP_KERNEL);
> > +       if (!path)
> > +               return -ENOMEM;
> > +
> > +       scnprintf(path, len, "%s%s", tcon->tree_name, full_path);
> > +       ref_path = dfs_cache_canonical_path(path + 1, cifs_sb->local_nls,
> > +                                           cifs_remap(cifs_sb));
> > +       kfree(path);
> > +
> > +       if (IS_ERR(ref_path)) {
> > +               if (PTR_ERR(ref_path) != -EINVAL)
> > +                       return PTR_ERR(ref_path);
> > +       } else {
> > +               struct dfs_info3_param *refs = NULL;
> > +               int num_refs = 0;
> > +
> > +               /*
> > +                * XXX: we are not using dfs_cache_find() here because we might
> > +                * end filling all the DFS cache and thus potentially
> > +                * removing cached DFS targets that the client would eventually
> > +                * need during failover.
> > +                */
> > +               if (ses->server->ops->get_dfs_refer &&
> > +                   !ses->server->ops->get_dfs_refer(xid, ses, ref_path, &refs,
> > +                                                    &num_refs, cifs_sb->local_nls,
> > +                                                    cifs_remap(cifs_sb)))
> > +                       *islink = refs[0].server_type == DFS_TYPE_LINK;
> > +               free_dfs_info_array(refs, num_refs);
> > +               kfree(ref_path);
> > +       }
> > +       return 0;
> > +}
> >  #endif
> > diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
> > index 37b4cd59245d..9b956294e864 100644
> > --- a/fs/cifs/smb2inode.c
> > +++ b/fs/cifs/smb2inode.c
> > @@ -527,12 +527,13 @@ int smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon,
> >                          struct cifs_sb_info *cifs_sb, const char *full_path,
> >                          struct cifs_open_info_data *data, bool *adjust_tz, bool *reparse)
> >  {
> > -       int rc;
> >         __u32 create_options = 0;
> >         struct cifsFileInfo *cfile;
> >         struct cached_fid *cfid = NULL;
> >         struct kvec err_iov[3] = {};
> >         int err_buftype[3] = {};
> > +       bool islink;
> > +       int rc, rc2;
> >
> >         *adjust_tz = false;
> >         *reparse = false;
> > @@ -580,15 +581,15 @@ int smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon,
> >                                               SMB2_OP_QUERY_INFO, cfile, NULL, NULL,
> >                                               NULL, NULL);
> >                         goto out;
> > -               } else if (rc != -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) &&
> > -                          hdr->Status == STATUS_OBJECT_NAME_INVALID) {
> > -                       /*
> > -                        * Handle weird Windows SMB server behaviour. It responds with
> > -                        * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request
> > -                        * for "\<server>\<dfsname>\<linkpath>" DFS reference,
> > -                        * where <dfsname> contains non-ASCII unicode symbols.
> > -                        */
> > -                       rc = -EREMOTE;
> > +               } else if (rc != -EREMOTE && hdr->Status == STATUS_OBJECT_NAME_INVALID) {
> > +                       rc2 = cifs_inval_name_dfs_link_error(xid, tcon, cifs_sb,
> > +                                                            full_path, &islink);
> > +                       if (rc2) {
> > +                               rc = rc2;
> > +                               goto out;
> > +                       }
> > +                       if (islink)
> > +                               rc = -EREMOTE;
> >                 }
> >                 if (rc == -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && cifs_sb &&
> >                     (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS))
> > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > index f79b075f2992..6dfb865ee9d7 100644
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -796,7 +796,6 @@ static int
> >  smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
> >                         struct cifs_sb_info *cifs_sb, const char *full_path)
> >  {
> > -       int rc;
> >         __le16 *utf16_path;
> >         __u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
> >         int err_buftype = CIFS_NO_BUFFER;
> > @@ -804,6 +803,8 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
> >         struct kvec err_iov = {};
> >         struct cifs_fid fid;
> >         struct cached_fid *cfid;
> > +       bool islink;
> > +       int rc, rc2;
> >
> >         rc = open_cached_dir(xid, tcon, full_path, cifs_sb, true, &cfid);
> >         if (!rc) {
> > @@ -833,15 +834,17 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
> >
> >                 if (unlikely(!hdr || err_buftype == CIFS_NO_BUFFER))
> >                         goto out;
> > -               /*
> > -                * Handle weird Windows SMB server behaviour. It responds with
> > -                * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request
> > -                * for "\<server>\<dfsname>\<linkpath>" DFS reference,
> > -                * where <dfsname> contains non-ASCII unicode symbols.
> > -                */
> > -               if (rc != -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) &&
> > -                   hdr->Status == STATUS_OBJECT_NAME_INVALID)
> > -                       rc = -EREMOTE;
> > +
> > +               if (rc != -EREMOTE && hdr->Status == STATUS_OBJECT_NAME_INVALID) {
> > +                       rc2 = cifs_inval_name_dfs_link_error(xid, tcon, cifs_sb,
> > +                                                            full_path, &islink);
> > +                       if (rc2) {
> > +                               rc = rc2;
> > +                               goto out;
> > +                       }
> > +                       if (islink)
> > +                               rc = -EREMOTE;
> > +               }
> >                 if (rc == -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && cifs_sb &&
> >                     (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS))
> >                         rc = -EOPNOTSUPP;
> > --
> > 2.39.2
> >



-- 
Thanks,

Steve




[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux