| Message ID | 20240915194545.14779-2-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs: Improve client SFU support for special files (2) | expand |
merged into cifs-2.6.git for-next pending review/testing On Sun, Sep 15, 2024 at 2:46 PM Pali Rohár <pali@kernel.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@kernel.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 > >
Hi Pali, On 09/15, Pali Rohár 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@kernel.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; Can't S_IFLNK be handled somewhere else/other function? mknod doesn't support S_IFLNK, so this seems out of place. And even though it's unreachable (AFAICS), cifs_sfu_make_node() calls this with @symname == NULL (caught with gcc -fanalyzer). Cheers, Enzo > 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;
On Friday 27 September 2024 14:54:31 Enzo Matsumiya wrote: > Hi Pali, > > On 09/15, Pali Rohár 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@kernel.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; > > Can't S_IFLNK be handled somewhere else/other function? mknod doesn't > support S_IFLNK, so this seems out of place. > > And even though it's unreachable (AFAICS), cifs_sfu_make_node() calls > this with @symname == NULL (caught with gcc -fanalyzer). > > > Cheers, > > Enzo Hello! As SFU-style special files have same format, I just extended this function which handles all SFU types, to handle also symlink. I wanted to reuse existing function instead of copying and duplicating code. Parameter symname is used only for S_IFLNK, so this should be safe. And as you pointed out mknod does not support S_IFLNK, so mknod code path would not call __cifs_sfu_make_node() function with S_IFLNK argument. So I think that there is not issue. But if you want to refactor this code, do you have an idea what to do? > > 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;
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;
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@kernel.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(-)