On Sun, 2 Oct 2022 05:46:28 +0300, Atte Heikkilä wrote:
> Share config response may contain the share name without casefolding as
> it is known to the user space daemon. When it is present, casefold and
> compare it to the share name the share config request was made with. If
> they differ, we have a share config which is incompatible with the way
> share config caching is done. This is the case when CONFIG_UNICODE is
> not set, the share name contains non-ASCII characters, and those non-
> ASCII characters do not match those in the share name known to user
> space. In other words, when CONFIG_UNICODE is not set, UTF-8 share
> names now work but are only case-insensitive in the ASCII range.
>
> Signed-off-by: Atte Heikkilä <atteh.mailbox@xxxxxxxxx>
> ---
> v4:
> - check for ksmbd_casefold_sharename() error with IS_ERR()
>
> v3:
> - removed initial strcmp() check since it could only save a call to
> ksmbd_casefold_sharename() for matching ASCII-only share names
>
> v2:
> - no changes were made
>
> fs/ksmbd/ksmbd_netlink.h | 3 ++-
> fs/ksmbd/mgmt/share_config.c | 20 +++++++++++++++++---
> fs/ksmbd/mgmt/share_config.h | 4 +++-
> fs/ksmbd/mgmt/tree_connect.c | 4 ++--
> fs/ksmbd/misc.c | 4 ++--
> fs/ksmbd/misc.h | 1 +
> 6 files changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/fs/ksmbd/ksmbd_netlink.h b/fs/ksmbd/ksmbd_netlink.h
> index e0cbcfa98c7e..ff07c67f4565 100644
> --- a/fs/ksmbd/ksmbd_netlink.h
> +++ b/fs/ksmbd/ksmbd_netlink.h
> @@ -163,7 +163,8 @@ struct ksmbd_share_config_response {
> __u16 force_directory_mode;
> __u16 force_uid;
> __u16 force_gid;
> - __u32 reserved[128]; /* Reserved room */
> + __s8 share_name[KSMBD_REQ_MAX_SHARE_NAME];
> + __u32 reserved[112]; /* Reserved room */
> __u32 veto_list_sz;
> __s8 ____payload[];
> };
> diff --git a/fs/ksmbd/mgmt/share_config.c b/fs/ksmbd/mgmt/share_config.c
> index 5d039704c23c..dfb4bb365891 100644
> --- a/fs/ksmbd/mgmt/share_config.c
> +++ b/fs/ksmbd/mgmt/share_config.c
> @@ -16,6 +16,7 @@
> #include "user_config.h"
> #include "user_session.h"
> #include "../transport_ipc.h"
> +#include "../misc.h"
>
> #define SHARE_HASH_BITS 3
> static DEFINE_HASHTABLE(shares_table, SHARE_HASH_BITS);
> @@ -119,7 +120,8 @@ static int parse_veto_list(struct ksmbd_share_config *share,
> return 0;
> }
>
> -static struct ksmbd_share_config *share_config_request(const char *name)
> +static struct ksmbd_share_config *share_config_request(struct unicode_map *um,
> + const char *name)
> {
> struct ksmbd_share_config_response *resp;
> struct ksmbd_share_config *share = NULL;
> @@ -133,6 +135,17 @@ static struct ksmbd_share_config *share_config_request(const char *name)
> if (resp->flags == KSMBD_SHARE_FLAG_INVALID)
> goto out;
>
> + if (*resp->share_name) {
> + char *cf_resp_name;
> + bool equal;
> +
> + cf_resp_name = ksmbd_casefold_sharename(um, resp->share_name);
> + equal = !IS_ERR(cf_resp_name) && !strcmp(cf_resp_name, name);
> + kfree(cf_resp_name);
Well, kfree() is *not* a no-op for ERR_PTR() like it is for NULL so
this patch is not good either. At least I'm running out of ways to get
this wrong.
> + if (!equal)
> + goto out;
> + }
> +
> share = kzalloc(sizeof(struct ksmbd_share_config), GFP_KERNEL);
> if (!share)
> goto out;
> @@ -190,7 +203,8 @@ static struct ksmbd_share_config *share_config_request(const char *name)
> return share;
> }
>
> -struct ksmbd_share_config *ksmbd_share_config_get(const char *name)
> +struct ksmbd_share_config *ksmbd_share_config_get(struct unicode_map *um,
> + const char *name)
> {
> struct ksmbd_share_config *share;
>
> @@ -202,7 +216,7 @@ struct ksmbd_share_config *ksmbd_share_config_get(const char *name)
>
> if (share)
> return share;
> - return share_config_request(name);
> + return share_config_request(um, name);
> }
>
> bool ksmbd_share_veto_filename(struct ksmbd_share_config *share,
> diff --git a/fs/ksmbd/mgmt/share_config.h b/fs/ksmbd/mgmt/share_config.h
> index 7f7e89ecfe61..3fd338293942 100644
> --- a/fs/ksmbd/mgmt/share_config.h
> +++ b/fs/ksmbd/mgmt/share_config.h
> @@ -9,6 +9,7 @@
> #include <linux/workqueue.h>
> #include <linux/hashtable.h>
> #include <linux/path.h>
> +#include <linux/unicode.h>
>
> struct ksmbd_share_config {
> char *name;
> @@ -74,7 +75,8 @@ static inline void ksmbd_share_config_put(struct ksmbd_share_config *share)
> __ksmbd_share_config_put(share);
> }
>
> -struct ksmbd_share_config *ksmbd_share_config_get(const char *name);
> +struct ksmbd_share_config *ksmbd_share_config_get(struct unicode_map *um,
> + const char *name);
> bool ksmbd_share_veto_filename(struct ksmbd_share_config *share,
> const char *filename);
> #endif /* __SHARE_CONFIG_MANAGEMENT_H__ */
> diff --git a/fs/ksmbd/mgmt/tree_connect.c b/fs/ksmbd/mgmt/tree_connect.c
> index 867c0286b901..8ce17b3fb8da 100644
> --- a/fs/ksmbd/mgmt/tree_connect.c
> +++ b/fs/ksmbd/mgmt/tree_connect.c
> @@ -26,7 +26,7 @@ ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess,
> struct sockaddr *peer_addr;
> int ret;
>
> - sc = ksmbd_share_config_get(share_name);
> + sc = ksmbd_share_config_get(conn->um, share_name);
> if (!sc)
> return status;
>
> @@ -61,7 +61,7 @@ ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess,
> struct ksmbd_share_config *new_sc;
>
> ksmbd_share_config_del(sc);
> - new_sc = ksmbd_share_config_get(share_name);
> + new_sc = ksmbd_share_config_get(conn->um, share_name);
> if (!new_sc) {
> pr_err("Failed to update stale share config\n");
> status.ret = -ESTALE;
> diff --git a/fs/ksmbd/misc.c b/fs/ksmbd/misc.c
> index 28459b1efaa8..9e8afaa686e3 100644
> --- a/fs/ksmbd/misc.c
> +++ b/fs/ksmbd/misc.c
> @@ -227,7 +227,7 @@ void ksmbd_conv_path_to_windows(char *path)
> strreplace(path, '/', '\\');
> }
>
> -static char *casefold_sharename(struct unicode_map *um, const char *name)
> +char *ksmbd_casefold_sharename(struct unicode_map *um, const char *name)
> {
> char *cf_name;
> int cf_len;
> @@ -273,7 +273,7 @@ char *ksmbd_extract_sharename(struct unicode_map *um, const char *treename)
> name = (pos + 1);
>
> /* caller has to free the memory */
> - return casefold_sharename(um, name);
> + return ksmbd_casefold_sharename(um, name);
> }
>
> /**
> diff --git a/fs/ksmbd/misc.h b/fs/ksmbd/misc.h
> index cc72f4e6baf2..1facfcd21200 100644
> --- a/fs/ksmbd/misc.h
> +++ b/fs/ksmbd/misc.h
> @@ -20,6 +20,7 @@ int get_nlink(struct kstat *st);
> void ksmbd_conv_path_to_unix(char *path);
> void ksmbd_strip_last_slash(char *path);
> void ksmbd_conv_path_to_windows(char *path);
> +char *ksmbd_casefold_sharename(struct unicode_map *um, const char *name);
> char *ksmbd_extract_sharename(struct unicode_map *um, const char *treename);
> char *convert_to_unix_name(struct ksmbd_share_config *share, const char *name);
>
> --
> 2.37.3
>
>
