ksmbd-tools: don't treat global guest as share guest on config reload

When the global guest account is specified in ksmbd.conf and a share
guest account is not, the global guest account definition is mistaken
for the share guest account definition on config reload. This happens
due to the `guest account' global parameter and the `guest account'
share parameter having the same name, together with the implemented way
share parameter definitions in the global section become the implicit
defaults for all shares. Fix this by removing `guest account' from the
parameters that are to be applied to all shares. Note that on initial
startup, i.e. not on config reload, it is done in global_conf_create().

On both initial startup and config reload, create the global guest
account in global_conf_fixup_missing(). This fixes another bug, where
the global guest account is not recreated on config reload if both the
global guest account and the share guest account are user-given, caused
partially also by the first bug identified and fixed here.

Finally, add usm_add_guest_account() and use it when needing to add a
guest account, whether it be the global one or the share one. Note that
it is purposefully identical to usm_add_new_user() when it comes to the
use and lifetime of `name'. Also, tweak the debug log message when user
already exists in usm_add_new_user() so that it is less ambiguous when
seen with the following kill_ksmbd_user() debug log message.

Signed-off-by: Atte Heikkilä <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>

include/management/user.h

@@ -48,6 +48,7 @@ int usm_update_user_password(struct ksmbd_user *user, char *pass);
 
 int usm_add_new_user(char *name, char *pwd);
 int usm_add_update_user_from_pwdentry(char *data);
+int usm_add_guest_account(char *name);
 int usm_add_subauth_global_conf(char *data);
 
 void usm_remove_all_users(void);

tools/config_parser.c

@@ -368,28 +368,6 @@ void cp_group_kv_list_free(char **list)
 	g_strfreev(list);
 }
 
-static int cp_add_global_guest_account(gpointer _v)
-{
-	struct ksmbd_user *user;
-
-	if (usm_add_new_user(cp_get_group_kv_string(_v),
-			     g_strdup("NULL"))) {
-		pr_err("Unable to add new user `%s'\n", (char *) _v);
-		return -ENOMEM;
-	}
-
-	user = usm_lookup_user(_v);
-	if (!user) {
-		pr_err("Unable to find user `%s'\n", (char *) _v);
-		return -EINVAL;
-	}
-
-	set_user_flag(user, KSMBD_USER_FLAG_GUEST_ACCOUNT);
-	put_ksmbd_user(user);
-	global_conf.guest_account = cp_get_group_kv_string(_v);
-	return 0;
-}
-
 static gboolean global_group_kv(gpointer _k, gpointer _v, gpointer user_data)
 {
 	if (!cp_key_cmp(_k, "server string")) {
@@ -423,8 +401,7 @@ static gboolean global_group_kv(gpointer _k, gpointer _v, gpointer user_data)
 	}
 
 	if (!cp_key_cmp(_k, "guest account")) {
-		if (cp_add_global_guest_account(_v))
-			pr_err("Unable to add global guest account\n");
+		global_conf.guest_account = cp_get_group_kv_string(_v);
 		return TRUE;
 	}
 
@@ -608,13 +585,12 @@ static void global_conf_update(struct smbconf_group *group)
 	if (!global_group)
 		return;
 
+	g_hash_table_remove(global_group->kv, "guest account");
 	g_hash_table_foreach(global_group->kv, append_key_value, group->kv);
 }
 
 static void global_conf_fixup_missing(void)
 {
-	struct ksmbd_user *user;
-
 	/*
 	 * Set default global parameters which were not specified
 	 * in smb.conf
@@ -637,11 +613,15 @@ static void global_conf_fixup_missing(void)
 	if (global_conf.sessions_cap <= 0)
 		global_conf.sessions_cap = KSMBD_CONF_DEFAULT_SESS_CAP;
 
-	user = usm_lookup_user(global_conf.guest_account);
-	if (user)
-		put_ksmbd_user(user);
-	else if (cp_add_global_guest_account(KSMBD_CONF_DEFAULT_GUEST_ACCOUNT))
-		pr_err("Unable to add default global guest account\n");
+	if (!global_conf.guest_account)
+		global_conf.guest_account =
+			cp_get_group_kv_string(
+					KSMBD_CONF_DEFAULT_GUEST_ACCOUNT);
+
+	if (usm_add_guest_account(g_strdup(global_conf.guest_account))) {
+		g_free(global_conf.guest_account);
+		global_conf.guest_account = NULL;
+	}
 }
 
 static void groups_callback(gpointer _k, gpointer _v, gpointer user_data)

tools/management/share.c

@@ -400,21 +400,11 @@ static void process_group_kv(gpointer _k, gpointer _v, gpointer user_data)
 	}
 
 	if (shm_share_config(k, KSMBD_SHARE_CONF_GUEST_ACCOUNT)) {
-		struct ksmbd_user *user;
-
-		if (usm_add_new_user(cp_get_group_kv_string(_v),
-				     g_strdup("NULL"))) {
-			pr_err("Unable to add guest account\n");
+		if (usm_add_guest_account(cp_get_group_kv_string(v))) {
 			set_share_flag(share, KSMBD_SHARE_FLAG_INVALID);
 			return;
 		}
-
-		user = usm_lookup_user(_v);
-		if (user) {
-			set_user_flag(user, KSMBD_USER_FLAG_GUEST_ACCOUNT);
-			put_ksmbd_user(user);
-		}
-		share->guest_account = cp_get_group_kv_string(_v);
+		share->guest_account = cp_get_group_kv_string(v);
 		if (!share->guest_account)
 			set_share_flag(share, KSMBD_SHARE_FLAG_INVALID);
 		return;

tools/management/user.c

@@ -184,7 +184,7 @@ int usm_add_new_user(char *name, char *pwd)
 	g_rw_lock_writer_lock(&users_table_lock);
 	if (__usm_lookup_user(name)) {
 		g_rw_lock_writer_unlock(&users_table_lock);
-		pr_debug("User `%s' already exists\n", name);
+		pr_debug("New user `%s' already exists\n", name);
 		kill_ksmbd_user(user);
 		return 0;
 	}
@@ -233,6 +233,25 @@ int usm_add_update_user_from_pwdentry(char *data)
 	return usm_add_new_user(name, pwd);
 }
 
+int usm_add_guest_account(char *name)
+{
+	g_autofree char *account = g_strdup(name);
+	struct ksmbd_user *user;
+
+	if (usm_add_new_user(name, g_strdup("NULL"))) {
+		pr_err("Unable to add guest account `%s'\n", account);
+		return -EINVAL;
+	}
+
+	user = usm_lookup_user(account);
+	if (!user)
+		return -EINVAL;
+
+	set_user_flag(user, KSMBD_USER_FLAG_GUEST_ACCOUNT);
+	put_ksmbd_user(user);
+	return 0;
+}
+
 int usm_add_subauth_global_conf(char *data)
 {
 	char *pos = data;