lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAH2r5mv0fZTiczADy2Ym65unER3kQoXTSaA_Q_9Jd72YQhusbw@mail.gmail.com>
Date: Tue, 1 Oct 2024 21:55:07 -0500
From: Steve French <smfrench@...il.com>
To: wangrong <wangrong@...ontech.com>
Cc: sfrench@...ba.org, pc@...guebit.com, ronniesahlberg@...il.com, 
	sprasad@...rosoft.com, tom@...pey.com, linux-cifs@...r.kernel.org, 
	samba-technical@...ts.samba.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] smb: client: use actual path when queryfs

Merged into cifs-2.6.git for-next tentatively but want to do some more
testing on this (and any additional review comments would be welcome)

On Tue, Oct 1, 2024 at 9:33 PM Steve French <smfrench@...il.com> wrote:
>
> Paulo just found this potentially important fix in email (it had gotten missed).
>
> The one suspicious thing about this though ... we should have some
> code paths where we would use the cached root handle for statfs
> (which is preferable to doing an open of a new handle, since it is
> already open we don't risk an error coming back on open).
>
> Any ideas whether we also need additional changes to use the cached
> root handle better in statfs (since in most cases to
> Windows we would expect to have that)?
>
>
> On Thu, Jun 20, 2024 at 3:54 AM wangrong <wangrong@...ontech.com> wrote:
> >
> > Due to server permission control, the client does not have access to
> > the shared root directory, but can access subdirectories normally, so
> > users usually mount the shared subdirectories directly. In this case,
> > queryfs should use the actual path instead of the root directory to
> > avoid the call returning an error (EACCES).
> >
> > Signed-off-by: wangrong <wangrong@...ontech.com>
> > ---
> >  fs/smb/client/cifsfs.c   | 13 ++++++++++++-
> >  fs/smb/client/cifsglob.h |  2 +-
> >  fs/smb/client/smb1ops.c  |  2 +-
> >  fs/smb/client/smb2ops.c  | 19 ++++++++++++-------
> >  4 files changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> > index bb86fc064..a4d59f0f5 100644
> > --- a/fs/smb/client/cifsfs.c
> > +++ b/fs/smb/client/cifsfs.c
> > @@ -313,8 +313,17 @@ cifs_statfs(struct dentry *dentry, struct kstatfs *buf)
> >         struct TCP_Server_Info *server = tcon->ses->server;
> >         unsigned int xid;
> >         int rc = 0;
> > +       const char *full_path;
> > +       void *page;
> >
> >         xid = get_xid();
> > +       page = alloc_dentry_path();
> > +
> > +       full_path = build_path_from_dentry(dentry, page);
> > +       if (IS_ERR(full_path)) {
> > +               rc = PTR_ERR(full_path);
> > +               goto statfs_out;
> > +       }
> >
> >         if (le32_to_cpu(tcon->fsAttrInfo.MaxPathNameComponentLength) > 0)
> >                 buf->f_namelen =
> > @@ -330,8 +339,10 @@ cifs_statfs(struct dentry *dentry, struct kstatfs *buf)
> >         buf->f_ffree = 0;       /* unlimited */
> >
> >         if (server->ops->queryfs)
> > -               rc = server->ops->queryfs(xid, tcon, cifs_sb, buf);
> > +               rc = server->ops->queryfs(xid, tcon, full_path, cifs_sb, buf);
> >
> > +statfs_out:
> > +       free_dentry_path(page);
> >         free_xid(xid);
> >         return rc;
> >  }
> > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> > index 73482734a..d3118d748 100644
> > --- a/fs/smb/client/cifsglob.h
> > +++ b/fs/smb/client/cifsglob.h
> > @@ -483,7 +483,7 @@ struct smb_version_operations {
> >                         __u16 net_fid, struct cifsInodeInfo *cifs_inode);
> >         /* query remote filesystem */
> >         int (*queryfs)(const unsigned int, struct cifs_tcon *,
> > -                      struct cifs_sb_info *, struct kstatfs *);
> > +                      const char *, struct cifs_sb_info *, struct kstatfs *);
> >         /* send mandatory brlock to the server */
> >         int (*mand_lock)(const unsigned int, struct cifsFileInfo *, __u64,
> >                          __u64, __u32, int, int, bool);
> > diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
> > index 212ec6f66..e3a195824 100644
> > --- a/fs/smb/client/smb1ops.c
> > +++ b/fs/smb/client/smb1ops.c
> > @@ -909,7 +909,7 @@ cifs_oplock_response(struct cifs_tcon *tcon, __u64 persistent_fid,
> >
> >  static int
> >  cifs_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> > -            struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> > +            const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> >  {
> >         int rc = -EOPNOTSUPP;
> >
> > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > index c8e536540..bb7194386 100644
> > --- a/fs/smb/client/smb2ops.c
> > +++ b/fs/smb/client/smb2ops.c
> > @@ -2784,7 +2784,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
> >
> >  static int
> >  smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> > -            struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> > +            const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> >  {
> >         struct smb2_query_info_rsp *rsp;
> >         struct smb2_fs_full_size_info *info = NULL;
> > @@ -2793,7 +2793,7 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> >         int rc;
> >
> >
> > -       rc = smb2_query_info_compound(xid, tcon, "",
> > +       rc = smb2_query_info_compound(xid, tcon, path,
> >                                       FILE_READ_ATTRIBUTES,
> >                                       FS_FULL_SIZE_INFORMATION,
> >                                       SMB2_O_INFO_FILESYSTEM,
> > @@ -2821,28 +2821,33 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> >
> >  static int
> >  smb311_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
> > -              struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> > +              const char *path, struct cifs_sb_info *cifs_sb, struct kstatfs *buf)
> >  {
> >         int rc;
> > -       __le16 srch_path = 0; /* Null - open root of share */
> > +       __le16 *utf16_path = NULL;
> >         u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
> >         struct cifs_open_parms oparms;
> >         struct cifs_fid fid;
> >
> >         if (!tcon->posix_extensions)
> > -               return smb2_queryfs(xid, tcon, cifs_sb, buf);
> > +               return smb2_queryfs(xid, tcon, path, cifs_sb, buf);
> >
> >         oparms = (struct cifs_open_parms) {
> >                 .tcon = tcon,
> > -               .path = "",
> > +               .path = path,
> >                 .desired_access = FILE_READ_ATTRIBUTES,
> >                 .disposition = FILE_OPEN,
> >                 .create_options = cifs_create_options(cifs_sb, 0),
> >                 .fid = &fid,
> >         };
> >
> > -       rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL,
> > +       utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
> > +       if (utf16_path == NULL)
> > +               return -ENOMEM;
> > +
> > +       rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL,
> >                        NULL, NULL);
> > +       kfree(utf16_path);
> >         if (rc)
> >                 return rc;
> >
> > --
> > 2.20.1
> >
> >
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ