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: <CAH2r5msn4JgezFNPmgfWA6Ti1SzorvfhXXkk747bQ4R4QjNkCQ@mail.gmail.com>
Date: Sun, 15 Sep 2024 16:15:09 -0500
From: Steve French <smfrench@...il.com>
To: Pali Rohár <pali@...nel.org>
Cc: Steve French <sfrench@...ba.org>, Paulo Alcantara <pc@...guebit.com>, 
	Ronnie Sahlberg <ronniesahlberg@...il.com>, linux-cifs@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] cifs: Add support for creating SFU symlinks

merged into cifs-2.6.git for-next pending review/testing

On Sun, Sep 15, 2024 at 2:46 PM Pali Rohár <pali@...nel.org> wrote:
>
> Linux cifs client can already detect SFU symlinks and reads it content
> (target location). But currently is not able to create new symlink. So
> implement this missing support.
>
> When 'sfu' mount option is specified and 'mfsymlinks' is not specified then
> create new symlinks in SFU-style. This will provide full SFU compatibility
> of symlinks when mounting cifs share with 'sfu' option. 'mfsymlinks' option
> override SFU for better Apple compatibility as explained in fs_context.c
> file in smb3_update_mnt_flags() function.
>
> Extend __cifs_sfu_make_node() function, which now can handle also S_IFLNK
> type and refactor structures passed to sync_write() in this function, by
> splitting SFU type and SFU data from original combined struct win_dev as
> combined fixed-length struct cannot be used for variable-length symlinks.
>
> Signed-off-by: Pali Rohár <pali@...nel.org>
> ---
>  fs/smb/client/cifspdu.h    |  6 ---
>  fs/smb/client/cifsproto.h  |  4 ++
>  fs/smb/client/fs_context.c | 13 ++++---
>  fs/smb/client/link.c       |  3 ++
>  fs/smb/client/smb2ops.c    | 80 +++++++++++++++++++++++++++++---------
>  5 files changed, 77 insertions(+), 29 deletions(-)
>
> diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
> index a2072ab9e586..c3b6263060b0 100644
> --- a/fs/smb/client/cifspdu.h
> +++ b/fs/smb/client/cifspdu.h
> @@ -2573,12 +2573,6 @@ typedef struct {
>  } __attribute__((packed)) FIND_FILE_STANDARD_INFO; /* level 0x1 FF resp data */
>
>
> -struct win_dev {
> -       unsigned char type[8]; /* IntxCHR or IntxBLK or LnxFIFO or LnxSOCK */
> -       __le64 major;
> -       __le64 minor;
> -} __attribute__((packed));
> -
>  struct fea {
>         unsigned char EA_flags;
>         __u8 name_len;
> diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> index 497bf3c447bc..791bddac0396 100644
> --- a/fs/smb/client/cifsproto.h
> +++ b/fs/smb/client/cifsproto.h
> @@ -676,6 +676,10 @@ char *extract_sharename(const char *unc);
>  int parse_reparse_point(struct reparse_data_buffer *buf,
>                         u32 plen, struct cifs_sb_info *cifs_sb,
>                         bool unicode, struct cifs_open_info_data *data);
> +int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> +                        struct dentry *dentry, struct cifs_tcon *tcon,
> +                        const char *full_path, umode_t mode, dev_t dev,
> +                        const char *symname);
>  int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>                        struct dentry *dentry, struct cifs_tcon *tcon,
>                        const char *full_path, umode_t mode, dev_t dev);
> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
> index bc926ab2555b..2f0c3894b0f7 100644
> --- a/fs/smb/client/fs_context.c
> +++ b/fs/smb/client/fs_context.c
> @@ -1896,14 +1896,17 @@ void smb3_update_mnt_flags(struct cifs_sb_info *cifs_sb)
>         if (ctx->mfsymlinks) {
>                 if (ctx->sfu_emul) {
>                         /*
> -                        * Our SFU ("Services for Unix" emulation does not allow
> -                        * creating symlinks but does allow reading existing SFU
> -                        * symlinks (it does allow both creating and reading SFU
> -                        * style mknod and FIFOs though). When "mfsymlinks" and
> +                        * Our SFU ("Services for Unix") emulation allows now
> +                        * creating new and reading existing SFU symlinks.
> +                        * Older Linux kernel versions were not able to neither
> +                        * read existing nor create new SFU symlinks. But
> +                        * creating and reading SFU style mknod and FIFOs was
> +                        * supported for long time. When "mfsymlinks" and
>                          * "sfu" are both enabled at the same time, it allows
>                          * reading both types of symlinks, but will only create
>                          * them with mfsymlinks format. This allows better
> -                        * Apple compatibility (probably better for Samba too)
> +                        * Apple compatibility, compatibility with older Linux
> +                        * kernel clients (probably better for Samba too)
>                          * while still recognizing old Windows style symlinks.
>                          */
>                         cifs_dbg(VFS, "mount options mfsymlinks and sfu both enabled\n");
> diff --git a/fs/smb/client/link.c b/fs/smb/client/link.c
> index 80099bbb333b..47ddeb7fa111 100644
> --- a/fs/smb/client/link.c
> +++ b/fs/smb/client/link.c
> @@ -606,6 +606,9 @@ cifs_symlink(struct mnt_idmap *idmap, struct inode *inode,
>         /* BB what if DFS and this volume is on different share? BB */
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) {
>                 rc = create_mf_symlink(xid, pTcon, cifs_sb, full_path, symname);
> +       } else if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL) {
> +               rc = __cifs_sfu_make_node(xid, inode, direntry, pTcon,
> +                                         full_path, S_IFLNK, 0, symname);
>  #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
>         } else if (pTcon->unix_ext) {
>                 rc = CIFSUnixCreateSymLink(xid, pTcon, full_path, symname,
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 9c2d065d3cc4..2c251e9a3a30 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -5055,9 +5055,10 @@ static int smb2_next_header(struct TCP_Server_Info *server, char *buf,
>         return 0;
>  }
>
> -static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
> +int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>                                 struct dentry *dentry, struct cifs_tcon *tcon,
> -                               const char *full_path, umode_t mode, dev_t dev)
> +                               const char *full_path, umode_t mode, dev_t dev,
> +                               const char *symname)
>  {
>         struct TCP_Server_Info *server = tcon->ses->server;
>         struct cifs_open_parms oparms;
> @@ -5065,30 +5066,64 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>         struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
>         struct cifs_fid fid;
>         unsigned int bytes_written;
> -       struct win_dev pdev = {};
> -       struct kvec iov[2];
> +       u8 type[8];
> +       int type_len = 0;
> +       struct {
> +               __le64 major;
> +               __le64 minor;
> +       } __packed pdev = {};
> +       __le16 *symname_utf16 = NULL;
> +       u8 *data = NULL;
> +       int data_len = 0;
> +       struct kvec iov[3];
>         __u32 oplock = server->oplocks ? REQ_OPLOCK : 0;
>         int rc;
>
>         switch (mode & S_IFMT) {
>         case S_IFCHR:
> -               memcpy(pdev.type, "IntxCHR\0", 8);
> +               type_len = 8;
> +               memcpy(type, "IntxCHR\0", type_len);
>                 pdev.major = cpu_to_le64(MAJOR(dev));
>                 pdev.minor = cpu_to_le64(MINOR(dev));
> +               data = (u8 *)&pdev;
> +               data_len = sizeof(pdev);
>                 break;
>         case S_IFBLK:
> -               memcpy(pdev.type, "IntxBLK\0", 8);
> +               type_len = 8;
> +               memcpy(type, "IntxBLK\0", type_len);
>                 pdev.major = cpu_to_le64(MAJOR(dev));
>                 pdev.minor = cpu_to_le64(MINOR(dev));
> +               data = (u8 *)&pdev;
> +               data_len = sizeof(pdev);
> +               break;
> +       case S_IFLNK:
> +               type_len = 8;
> +               memcpy(type, "IntxLNK\1", type_len);
> +               symname_utf16 = cifs_strndup_to_utf16(symname, strlen(symname),
> +                                                     &data_len, cifs_sb->local_nls,
> +                                                     NO_MAP_UNI_RSVD);
> +               if (!symname_utf16) {
> +                       rc = -ENOMEM;
> +                       goto out;
> +               }
> +               data_len -= 2; /* symlink is without trailing wide-nul */
> +               data = (u8 *)symname_utf16;
>                 break;
>         case S_IFSOCK:
> -               strscpy(pdev.type, "LnxSOCK");
> +               type_len = 8;
> +               strscpy(type, "LnxSOCK");
> +               data = (u8 *)&pdev;
> +               data_len = sizeof(pdev);
>                 break;
>         case S_IFIFO:
> -               strscpy(pdev.type, "LnxFIFO");
> +               type_len = 8;
> +               strscpy(type, "LnxFIFO");
> +               data = (u8 *)&pdev;
> +               data_len = sizeof(pdev);
>                 break;
>         default:
> -               return -EPERM;
> +               rc = -EPERM;
> +               goto out;
>         }
>
>         oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, GENERIC_WRITE,
> @@ -5098,17 +5133,26 @@ static int __cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>
>         rc = server->ops->open(xid, &oparms, &oplock, NULL);
>         if (rc)
> -               return rc;
> +               goto out;
>
> -       io_parms.pid = current->tgid;
> -       io_parms.tcon = tcon;
> -       io_parms.length = sizeof(pdev);
> -       iov[1].iov_base = &pdev;
> -       iov[1].iov_len = sizeof(pdev);
> +       if (type_len + data_len > 0) {
> +               io_parms.pid = current->tgid;
> +               io_parms.tcon = tcon;
> +               io_parms.length = type_len + data_len;
> +               iov[1].iov_base = type;
> +               iov[1].iov_len = type_len;
> +               iov[2].iov_base = data;
> +               iov[2].iov_len = data_len;
> +
> +               rc = server->ops->sync_write(xid, &fid, &io_parms,
> +                                            &bytes_written,
> +                                            iov, ARRAY_SIZE(iov)-1);
> +       }
>
> -       rc = server->ops->sync_write(xid, &fid, &io_parms,
> -                                    &bytes_written, iov, 1);
>         server->ops->close(xid, tcon, &fid);
> +
> +out:
> +       kfree(symname_utf16);
>         return rc;
>  }
>
> @@ -5120,7 +5164,7 @@ int cifs_sfu_make_node(unsigned int xid, struct inode *inode,
>         int rc;
>
>         rc = __cifs_sfu_make_node(xid, inode, dentry, tcon,
> -                                 full_path, mode, dev);
> +                                 full_path, mode, dev, NULL);
>         if (rc)
>                 return rc;
>
> --
> 2.20.1
>
>


-- 
Thanks,

Steve

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ