From eacc655e18d1dec9b50660d16a1ddeeb4d6c48f2 Mon Sep 17 00:00:00 2001 From: Cheng-Han Wu Date: Mon, 2 Oct 2023 13:32:03 +0800 Subject: [PATCH 1/7] ksmbd: Remove unused field in ksmbd_user struct fs/smb/server/mgmt/user_config.h:21: Remove the unused field 'failed_login_count' from the ksmbd_user struct. Signed-off-by: Cheng-Han Wu Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/mgmt/user_config.h | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/smb/server/mgmt/user_config.h b/fs/smb/server/mgmt/user_config.h index 6a44109617f14..e068a19fd9049 100644 --- a/fs/smb/server/mgmt/user_config.h +++ b/fs/smb/server/mgmt/user_config.h @@ -18,7 +18,6 @@ struct ksmbd_user { size_t passkey_sz; char *passkey; - unsigned int failed_login_count; }; static inline bool user_guest(struct ksmbd_user *user) From 1819a904299942b309f687cc0f08b123500aa178 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Mon, 9 Oct 2023 23:57:20 +0900 Subject: [PATCH 2/7] ksmbd: reorganize ksmbd_iov_pin_rsp() If ksmbd_iov_pin_rsp fail, io vertor should be rollback. This patch moves memory allocations to before setting the io vector to avoid rollbacks. Fixes: e2b76ab8b5c9 ("ksmbd: add support for read compound") Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/ksmbd_work.c | 43 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/fs/smb/server/ksmbd_work.c b/fs/smb/server/ksmbd_work.c index 51def3ca74c01..a2ed441e837ae 100644 --- a/fs/smb/server/ksmbd_work.c +++ b/fs/smb/server/ksmbd_work.c @@ -95,11 +95,28 @@ bool ksmbd_queue_work(struct ksmbd_work *work) return queue_work(ksmbd_wq, &work->work); } -static int ksmbd_realloc_iov_pin(struct ksmbd_work *work, void *ib, - unsigned int ib_len) +static inline void __ksmbd_iov_pin(struct ksmbd_work *work, void *ib, + unsigned int ib_len) { + work->iov[++work->iov_idx].iov_base = ib; + work->iov[work->iov_idx].iov_len = ib_len; + work->iov_cnt++; +} + +static int __ksmbd_iov_pin_rsp(struct ksmbd_work *work, void *ib, int len, + void *aux_buf, unsigned int aux_size) +{ + struct aux_read *ar; + int need_iov_cnt = 1; - if (work->iov_alloc_cnt <= work->iov_cnt) { + if (aux_size) { + need_iov_cnt++; + ar = kmalloc(sizeof(struct aux_read), GFP_KERNEL); + if (!ar) + return -ENOMEM; + } + + if (work->iov_alloc_cnt < work->iov_cnt + need_iov_cnt) { struct kvec *new; work->iov_alloc_cnt += 4; @@ -111,16 +128,6 @@ static int ksmbd_realloc_iov_pin(struct ksmbd_work *work, void *ib, work->iov = new; } - work->iov[++work->iov_idx].iov_base = ib; - work->iov[work->iov_idx].iov_len = ib_len; - work->iov_cnt++; - - return 0; -} - -static int __ksmbd_iov_pin_rsp(struct ksmbd_work *work, void *ib, int len, - void *aux_buf, unsigned int aux_size) -{ /* Plus rfc_length size on first iov */ if (!work->iov_idx) { work->iov[work->iov_idx].iov_base = work->response_buf; @@ -129,19 +136,13 @@ static int __ksmbd_iov_pin_rsp(struct ksmbd_work *work, void *ib, int len, work->iov_cnt++; } - ksmbd_realloc_iov_pin(work, ib, len); + __ksmbd_iov_pin(work, ib, len); inc_rfc1001_len(work->iov[0].iov_base, len); if (aux_size) { - struct aux_read *ar; - - ksmbd_realloc_iov_pin(work, aux_buf, aux_size); + __ksmbd_iov_pin(work, aux_buf, aux_size); inc_rfc1001_len(work->iov[0].iov_base, aux_size); - ar = kmalloc(sizeof(struct aux_read), GFP_KERNEL); - if (!ar) - return -ENOMEM; - ar->buf = aux_buf; list_add(&ar->entry, &work->aux_read_list); } From 3354db668808d5b6d7c5e0cb19ff4c9da4bb5e58 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Tue, 10 Oct 2023 00:04:10 +0900 Subject: [PATCH 3/7] ksmbd: fix kernel-doc comment of ksmbd_vfs_setxattr() Fix argument list that the kdoc format and script verified in ksmbd_vfs_setxattr(). fs/smb/server/vfs.c:929: warning: Function parameter or member 'path' not described in 'ksmbd_vfs_setxattr' Reported-by: kernel test robot Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/vfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index b5a5e50fc9ca3..2e37939a8ba08 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -919,7 +919,7 @@ ssize_t ksmbd_vfs_getxattr(struct mnt_idmap *idmap, /** * ksmbd_vfs_setxattr() - vfs helper for smb set extended attributes value * @idmap: idmap of the relevant mount - * @dentry: dentry to set XATTR at + * @path: path of dentry to set XATTR at * @attr_name: xattr name for setxattr * @attr_value: xattr value to set * @attr_size: size of xattr value From 807252f028c59b9a3bac4d62ad84761548c10f11 Mon Sep 17 00:00:00 2001 From: Marios Makassikis Date: Sat, 14 Oct 2023 12:48:25 +0900 Subject: [PATCH 4/7] ksmbd: fix recursive locking in vfs helpers Running smb2.rename test from Samba smbtorture suite against a kernel built with lockdep triggers a "possible recursive locking detected" warning. This is because mnt_want_write() is called twice with no mnt_drop_write() in between: -> ksmbd_vfs_mkdir() -> ksmbd_vfs_kern_path_create() -> kern_path_create() -> filename_create() -> mnt_want_write() -> mnt_want_write() Fix this by removing the mnt_want_write/mnt_drop_write calls from vfs helpers that call kern_path_create(). Full lockdep trace below: ============================================ WARNING: possible recursive locking detected 6.6.0-rc5 #775 Not tainted -------------------------------------------- kworker/1:1/32 is trying to acquire lock: ffff888005ac83f8 (sb_writers#5){.+.+}-{0:0}, at: ksmbd_vfs_mkdir+0xe1/0x410 but task is already holding lock: ffff888005ac83f8 (sb_writers#5){.+.+}-{0:0}, at: filename_create+0xb6/0x260 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(sb_writers#5); lock(sb_writers#5); *** DEADLOCK *** May be due to missing lock nesting notation 4 locks held by kworker/1:1/32: #0: ffff8880064e4138 ((wq_completion)ksmbd-io){+.+.}-{0:0}, at: process_one_work+0x40e/0x980 #1: ffff888005b0fdd0 ((work_completion)(&work->work)){+.+.}-{0:0}, at: process_one_work+0x40e/0x980 #2: ffff888005ac83f8 (sb_writers#5){.+.+}-{0:0}, at: filename_create+0xb6/0x260 #3: ffff8880057ce760 (&type->i_mutex_dir_key#3/1){+.+.}-{3:3}, at: filename_create+0x123/0x260 Cc: stable@vger.kernel.org Fixes: 40b268d384a2 ("ksmbd: add mnt_want_write to ksmbd vfs functions") Signed-off-by: Marios Makassikis Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/vfs.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index 2e37939a8ba08..1053127f71adf 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -173,10 +173,6 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode) return err; } - err = mnt_want_write(path.mnt); - if (err) - goto out_err; - mode |= S_IFREG; err = vfs_create(mnt_idmap(path.mnt), d_inode(path.dentry), dentry, mode, true); @@ -186,9 +182,7 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode) } else { pr_err("File(%s): creation failed (err:%d)\n", name, err); } - mnt_drop_write(path.mnt); -out_err: done_path_create(&path, dentry); return err; } @@ -219,10 +213,6 @@ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode) return err; } - err = mnt_want_write(path.mnt); - if (err) - goto out_err2; - idmap = mnt_idmap(path.mnt); mode |= S_IFDIR; err = vfs_mkdir(idmap, d_inode(path.dentry), dentry, mode); @@ -233,21 +223,19 @@ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode) dentry->d_name.len); if (IS_ERR(d)) { err = PTR_ERR(d); - goto out_err1; + goto out_err; } if (unlikely(d_is_negative(d))) { dput(d); err = -ENOENT; - goto out_err1; + goto out_err; } ksmbd_vfs_inherit_owner(work, d_inode(path.dentry), d_inode(d)); dput(d); } -out_err1: - mnt_drop_write(path.mnt); -out_err2: +out_err: done_path_create(&path, dentry); if (err) pr_err("mkdir(%s): creation failed (err:%d)\n", name, err); @@ -665,16 +653,11 @@ int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname, goto out3; } - err = mnt_want_write(newpath.mnt); - if (err) - goto out3; - err = vfs_link(oldpath.dentry, mnt_idmap(newpath.mnt), d_inode(newpath.dentry), dentry, NULL); if (err) ksmbd_debug(VFS, "vfs_link failed err %d\n", err); - mnt_drop_write(newpath.mnt); out3: done_path_create(&newpath, dentry); From ecce70cf17d91c3dd87a0c4ea00b2d1387729701 Mon Sep 17 00:00:00 2001 From: Kangjing Huang Date: Sat, 21 Oct 2023 03:42:07 -0400 Subject: [PATCH 5/7] ksmbd: fix missing RDMA-capable flag for IPoIB device in ksmbd_rdma_capable_netdev() Physical ib_device does not have an underlying net_device, thus its association with IPoIB net_device cannot be retrieved via ops.get_netdev() or ib_device_get_by_netdev(). ksmbd reads physical ib_device port GUID from the lower 16 bytes of the hardware addresses on IPoIB net_device and match its underlying ib_device using ib_find_gid() Signed-off-by: Kangjing Huang Acked-by: Namjae Jeon Reviewed-by: Tom Talpey Signed-off-by: Steve French --- fs/smb/server/transport_rdma.c | 40 +++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/fs/smb/server/transport_rdma.c b/fs/smb/server/transport_rdma.c index 3b269e1f523a1..c5629a68c8b73 100644 --- a/fs/smb/server/transport_rdma.c +++ b/fs/smb/server/transport_rdma.c @@ -2140,8 +2140,7 @@ static int smb_direct_ib_client_add(struct ib_device *ib_dev) if (ib_dev->node_type != RDMA_NODE_IB_CA) smb_direct_port = SMB_DIRECT_PORT_IWARP; - if (!ib_dev->ops.get_netdev || - !rdma_frwr_is_supported(&ib_dev->attrs)) + if (!rdma_frwr_is_supported(&ib_dev->attrs)) return 0; smb_dev = kzalloc(sizeof(*smb_dev), GFP_KERNEL); @@ -2241,17 +2240,38 @@ bool ksmbd_rdma_capable_netdev(struct net_device *netdev) for (i = 0; i < smb_dev->ib_dev->phys_port_cnt; i++) { struct net_device *ndev; - ndev = smb_dev->ib_dev->ops.get_netdev(smb_dev->ib_dev, - i + 1); - if (!ndev) - continue; + if (smb_dev->ib_dev->ops.get_netdev) { + ndev = smb_dev->ib_dev->ops.get_netdev( + smb_dev->ib_dev, i + 1); + if (!ndev) + continue; - if (ndev == netdev) { + if (ndev == netdev) { + dev_put(ndev); + rdma_capable = true; + goto out; + } dev_put(ndev); - rdma_capable = true; - goto out; + /* if ib_dev does not implement ops.get_netdev + * check for matching infiniband GUID in hw_addr + */ + } else if (netdev->type == ARPHRD_INFINIBAND) { + struct netdev_hw_addr *ha; + union ib_gid gid; + u32 port_num; + int ret; + + netdev_hw_addr_list_for_each( + ha, &netdev->dev_addrs) { + memcpy(&gid, ha->addr + 4, sizeof(gid)); + ret = ib_find_gid(smb_dev->ib_dev, &gid, + &port_num, NULL); + if (!ret) { + rdma_capable = true; + goto out; + } + } } - dev_put(ndev); } } out: From 0c180317c654a494fe429adbf7bc9b0793caf9e2 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Sat, 21 Oct 2023 21:01:20 +0900 Subject: [PATCH 6/7] ksmbd: add support for surrogate pair conversion ksmbd is missing supporting to convert filename included surrogate pair characters. It triggers a "file or folder does not exist" error in Windows client. [Steps to Reproduce for bug] 1. Create surrogate pair file touch $(echo -e '\xf0\x9d\x9f\xa3') touch $(echo -e '\xf0\x9d\x9f\xa4') 2. Try to open these files in ksmbd share through Windows client. This patch update unicode functions not to consider about surrogate pair (and IVS). Reviewed-by: Marios Makassikis Tested-by: Marios Makassikis Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/unicode.c | 187 +++++++++++++++++++++++++++++----------- 1 file changed, 138 insertions(+), 49 deletions(-) diff --git a/fs/smb/server/unicode.c b/fs/smb/server/unicode.c index 393dd4a7432b6..43ed29ee44ead 100644 --- a/fs/smb/server/unicode.c +++ b/fs/smb/server/unicode.c @@ -13,46 +13,10 @@ #include "unicode.h" #include "smb_common.h" -/* - * smb_utf16_bytes() - how long will a string be after conversion? - * @from: pointer to input string - * @maxbytes: don't go past this many bytes of input string - * @codepage: destination codepage - * - * Walk a utf16le string and return the number of bytes that the string will - * be after being converted to the given charset, not including any null - * termination required. Don't walk past maxbytes in the source buffer. - * - * Return: string length after conversion - */ -static int smb_utf16_bytes(const __le16 *from, int maxbytes, - const struct nls_table *codepage) -{ - int i; - int charlen, outlen = 0; - int maxwords = maxbytes / 2; - char tmp[NLS_MAX_CHARSET_SIZE]; - __u16 ftmp; - - for (i = 0; i < maxwords; i++) { - ftmp = get_unaligned_le16(&from[i]); - if (ftmp == 0) - break; - - charlen = codepage->uni2char(ftmp, tmp, NLS_MAX_CHARSET_SIZE); - if (charlen > 0) - outlen += charlen; - else - outlen++; - } - - return outlen; -} - /* * cifs_mapchar() - convert a host-endian char to proper char in codepage * @target: where converted character should be copied - * @src_char: 2 byte host-endian source character + * @from: host-endian source string * @cp: codepage to which character should be converted * @mapchar: should character be mapped according to mapchars mount option? * @@ -63,10 +27,13 @@ static int smb_utf16_bytes(const __le16 *from, int maxbytes, * Return: string length after conversion */ static int -cifs_mapchar(char *target, const __u16 src_char, const struct nls_table *cp, +cifs_mapchar(char *target, const __u16 *from, const struct nls_table *cp, bool mapchar) { int len = 1; + __u16 src_char; + + src_char = *from; if (!mapchar) goto cp_convert; @@ -104,12 +71,66 @@ cifs_mapchar(char *target, const __u16 src_char, const struct nls_table *cp, cp_convert: len = cp->uni2char(src_char, target, NLS_MAX_CHARSET_SIZE); - if (len <= 0) { - *target = '?'; - len = 1; - } + if (len <= 0) + goto surrogate_pair; goto out; + +surrogate_pair: + /* convert SURROGATE_PAIR and IVS */ + if (strcmp(cp->charset, "utf8")) + goto unknown; + len = utf16s_to_utf8s(from, 3, UTF16_LITTLE_ENDIAN, target, 6); + if (len <= 0) + goto unknown; + return len; + +unknown: + *target = '?'; + len = 1; + goto out; +} + +/* + * smb_utf16_bytes() - compute converted string length + * @from: pointer to input string + * @maxbytes: input string length + * @codepage: destination codepage + * + * Walk a utf16le string and return the number of bytes that the string will + * be after being converted to the given charset, not including any null + * termination required. Don't walk past maxbytes in the source buffer. + * + * Return: string length after conversion + */ +static int smb_utf16_bytes(const __le16 *from, int maxbytes, + const struct nls_table *codepage) +{ + int i, j; + int charlen, outlen = 0; + int maxwords = maxbytes / 2; + char tmp[NLS_MAX_CHARSET_SIZE]; + __u16 ftmp[3]; + + for (i = 0; i < maxwords; i++) { + ftmp[0] = get_unaligned_le16(&from[i]); + if (ftmp[0] == 0) + break; + for (j = 1; j <= 2; j++) { + if (i + j < maxwords) + ftmp[j] = get_unaligned_le16(&from[i + j]); + else + ftmp[j] = 0; + } + + charlen = cifs_mapchar(tmp, ftmp, codepage, 0); + if (charlen > 0) + outlen += charlen; + else + outlen++; + } + + return outlen; } /* @@ -139,12 +160,12 @@ cifs_mapchar(char *target, const __u16 src_char, const struct nls_table *cp, static int smb_from_utf16(char *to, const __le16 *from, int tolen, int fromlen, const struct nls_table *codepage, bool mapchar) { - int i, charlen, safelen; + int i, j, charlen, safelen; int outlen = 0; int nullsize = nls_nullsize(codepage); int fromwords = fromlen / 2; char tmp[NLS_MAX_CHARSET_SIZE]; - __u16 ftmp; + __u16 ftmp[3]; /* ftmp[3] = 3array x 2bytes = 6bytes UTF-16 */ /* * because the chars can be of varying widths, we need to take care @@ -155,9 +176,15 @@ static int smb_from_utf16(char *to, const __le16 *from, int tolen, int fromlen, safelen = tolen - (NLS_MAX_CHARSET_SIZE + nullsize); for (i = 0; i < fromwords; i++) { - ftmp = get_unaligned_le16(&from[i]); - if (ftmp == 0) + ftmp[0] = get_unaligned_le16(&from[i]); + if (ftmp[0] == 0) break; + for (j = 1; j <= 2; j++) { + if (i + j < fromwords) + ftmp[j] = get_unaligned_le16(&from[i + j]); + else + ftmp[j] = 0; + } /* * check to see if converting this character might make the @@ -172,6 +199,19 @@ static int smb_from_utf16(char *to, const __le16 *from, int tolen, int fromlen, /* put converted char into 'to' buffer */ charlen = cifs_mapchar(&to[outlen], ftmp, codepage, mapchar); outlen += charlen; + + /* + * charlen (=bytes of UTF-8 for 1 character) + * 4bytes UTF-8(surrogate pair) is charlen=4 + * (4bytes UTF-16 code) + * 7-8bytes UTF-8(IVS) is charlen=3+4 or 4+4 + * (2 UTF-8 pairs divided to 2 UTF-16 pairs) + */ + if (charlen == 4) + i++; + else if (charlen >= 5) + /* 5-6bytes UTF-8 */ + i += 2; } /* properly null-terminate string */ @@ -306,6 +346,9 @@ int smbConvertToUTF16(__le16 *target, const char *source, int srclen, char src_char; __le16 dst_char; wchar_t tmp; + wchar_t wchar_to[6]; /* UTF-16 */ + int ret; + unicode_t u; if (!mapchars) return smb_strtoUTF16(target, source, srclen, cp); @@ -348,11 +391,57 @@ int smbConvertToUTF16(__le16 *target, const char *source, int srclen, * if no match, use question mark, which at least in * some cases serves as wild card */ - if (charlen < 1) { - dst_char = cpu_to_le16(0x003f); - charlen = 1; + if (charlen > 0) + goto ctoUTF16; + + /* convert SURROGATE_PAIR */ + if (strcmp(cp->charset, "utf8")) + goto unknown; + if (*(source + i) & 0x80) { + charlen = utf8_to_utf32(source + i, 6, &u); + if (charlen < 0) + goto unknown; + } else + goto unknown; + ret = utf8s_to_utf16s(source + i, charlen, + UTF16_LITTLE_ENDIAN, + wchar_to, 6); + if (ret < 0) + goto unknown; + + i += charlen; + dst_char = cpu_to_le16(*wchar_to); + if (charlen <= 3) + /* 1-3bytes UTF-8 to 2bytes UTF-16 */ + put_unaligned(dst_char, &target[j]); + else if (charlen == 4) { + /* + * 4bytes UTF-8(surrogate pair) to 4bytes UTF-16 + * 7-8bytes UTF-8(IVS) divided to 2 UTF-16 + * (charlen=3+4 or 4+4) + */ + put_unaligned(dst_char, &target[j]); + dst_char = cpu_to_le16(*(wchar_to + 1)); + j++; + put_unaligned(dst_char, &target[j]); + } else if (charlen >= 5) { + /* 5-6bytes UTF-8 to 6bytes UTF-16 */ + put_unaligned(dst_char, &target[j]); + dst_char = cpu_to_le16(*(wchar_to + 1)); + j++; + put_unaligned(dst_char, &target[j]); + dst_char = cpu_to_le16(*(wchar_to + 2)); + j++; + put_unaligned(dst_char, &target[j]); } + continue; + +unknown: + dst_char = cpu_to_le16(0x003f); + charlen = 1; } + +ctoUTF16: /* * character may take more than one byte in the source string, * but will take exactly two bytes in the target string From 67797da8a4b82446d42c52b6ee1419a3100d78ff Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Sun, 29 Oct 2023 20:58:28 +0900 Subject: [PATCH 7/7] ksmbd: no need to wait for binded connection termination at logoff The connection could be binded to the existing session for Multichannel. session will be destroyed when binded connections are released. So no need to wait for that's connection at logoff. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/connection.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c index 4b38c3a285f60..b6fa1e285c401 100644 --- a/fs/smb/server/connection.c +++ b/fs/smb/server/connection.c @@ -167,23 +167,7 @@ void ksmbd_all_conn_set_status(u64 sess_id, u32 status) void ksmbd_conn_wait_idle(struct ksmbd_conn *conn, u64 sess_id) { - struct ksmbd_conn *bind_conn; - wait_event(conn->req_running_q, atomic_read(&conn->req_running) < 2); - - down_read(&conn_list_lock); - list_for_each_entry(bind_conn, &conn_list, conns_list) { - if (bind_conn == conn) - continue; - - if ((bind_conn->binding || xa_load(&bind_conn->sessions, sess_id)) && - !ksmbd_conn_releasing(bind_conn) && - atomic_read(&bind_conn->req_running)) { - wait_event(bind_conn->req_running_q, - atomic_read(&bind_conn->req_running) == 0); - } - } - up_read(&conn_list_lock); } int ksmbd_conn_write(struct ksmbd_work *work)