On Sun, 25 Sep 2022 22:18:19 +0900, Namjae Jeon wrote:
> 2022-09-24 11:23 GMT+09:00, Atte Heikkilä <atteh.mailbox@xxxxxxxxx>:
>> Case-insensitive file name lookups with __caseless_lookup() use
>> strncasecmp() for file name comparison. strncasecmp() assumes an
>> ISO8859-1-compatible encoding, which is not the case here as UTF-8
>> is always used. As such, use of strncasecmp() here produces correct
>> results only if both strings use characters in the ASCII range only.
>> Fix this by using utf8_strncasecmp() if CONFIG_UNICODE is set. On
>> failure or if CONFIG_UNICODE is not set, fallback to strncasecmp().
>> Also, as we are adding an include for `linux/unicode.h', include it
>> in `fs/ksmbd/connection.h' as well since it should be explicit there.
>>
>> Signed-off-by: Atte Heikkilä <atteh.mailbox@xxxxxxxxx>
>> ---
>> fs/ksmbd/connection.h | 1 +
>> fs/ksmbd/vfs.c | 20 +++++++++++++++++---
>> fs/ksmbd/vfs.h | 2 ++
>> 3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
>> index 41d96f5cef06..3643354a3fa7 100644
>> --- a/fs/ksmbd/connection.h
>> +++ b/fs/ksmbd/connection.h
>> @@ -14,6 +14,7 @@
>> #include <net/request_sock.h>
>> #include <linux/kthread.h>
>> #include <linux/nls.h>
>> +#include <linux/unicode.h>
>>
>> #include "smb_common.h"
>> #include "ksmbd_work.h"
>> diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c
>> index 4fcf96a01c16..a3269df7c7b3 100644
>> --- a/fs/ksmbd/vfs.c
>> +++ b/fs/ksmbd/vfs.c
>> @@ -1145,12 +1145,23 @@ static int __caseless_lookup(struct dir_context
>> *ctx, const char *name,
>> unsigned int d_type)
>> {
>> struct ksmbd_readdir_data *buf;
>> + int cmp;
> cmp should be initialized with -EINVAL to fallback strncasecmp() ?
Please see below for the explanation.
>
>>
>> buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
>>
>> if (buf->used != namlen)
>> return 0;
>> - if (!strncasecmp((char *)buf->private, name, namlen)) {
>> + if (IS_ENABLED(CONFIG_UNICODE) && buf->um) {
>> + const struct qstr q_buf = {.name = buf->private,
>> + .len = buf->used};
>> + const struct qstr q_name = {.name = name,
>> + .len = namlen};
>> +
>> + cmp = utf8_strncasecmp(buf->um, &q_buf, &q_name);
>> + }
>> + if (!(IS_ENABLED(CONFIG_UNICODE) && buf->um) || cmp < 0)
> I wonder why ->um is checked with CONFIG_UNICODE.
If !(IS_ENABLED(CONFIG_UNICODE) && buf->um) is true, then utf8_strncasecmp()
was not called. If !(IS_ENABLED(CONFIG_UNICODE) && buf->um) is false, then
utf8_strncasecmp() was called and we check for an error with `cmp < 0'.
Alternatively, `cmp' can be initialized to -EINVAL and then
!(IS_ENABLED(CONFIG_UNICODE) && buf->um) can be removed.
The latter is preferred, right?
>
> Thanks.
>> + cmp = strncasecmp((char *)buf->private, name, namlen);
>> + if (!cmp) {
>> memcpy((char *)buf->private, name, namlen);
>> buf->dirent_count = 1;
>> return -EEXIST;
>> @@ -1166,7 +1177,8 @@ static int __caseless_lookup(struct dir_context *ct> x,
>> const char *name,
>> *
>> * Return: 0 on success, otherwise error
>> */
>> -static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name,
>> size_t namelen)
>> +static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name,
>> + size_t namelen, struct unicode_map *um)
>> {
>> int ret;
>> struct file *dfilp;
>> @@ -1176,6 +1188,7 @@ static int ksmbd_vfs_lookup_in_dir(const struct pat> h
>> *dir, char *name, size_t na
>> .private = name,
>> .used = namelen,
>> .dirent_count = 0,
>> + .um = um,
>> };
>>
>> dfilp = dentry_open(dir, flags, current_cred());
>> @@ -1238,7 +1251,8 @@ int ksmbd_vfs_kern_path(struct ksmbd_work *work, ch> ar
>> *name,
>> break;
>>
>> err = ksmbd_vfs_lookup_in_dir(&parent, filename,
>> - filename_len);
>> + filename_len,
>> + work->conn->um);
>> path_put(&parent);
>> if (err)
>> goto out;
>> diff --git a/fs/ksmbd/vfs.h b/fs/ksmbd/vfs.h
>> index d7542a2dab52..593059ca8511 100644
>> --- a/fs/ksmbd/vfs.h
>> +++ b/fs/ksmbd/vfs.h
>> @@ -12,6 +12,7 @@
>> #include <linux/namei.h>
>> #include <uapi/linux/xattr.h>
>> #include <linux/posix_acl.h>
>> +#include <linux/unicode.h>
>>
>> #include "smbacl.h"
>> #include "xattr.h"
>> @@ -60,6 +61,7 @@ struct ksmbd_readdir_data {
>> unsigned int used;
>> unsigned int dirent_count;
>> unsigned int file_attr;
>> + struct unicode_map *um;
>> };
>>
>> /* ksmbd kstat wrapper to get valid create time when reading dir entry *> /
>> --
>> 2.37.3
>>
>>
>
