ksmbd-tools: improve mountd daemonization and signal handling

Currently, when mountd daemonizes, the PID file is *not* created before
the mountd manager parent exits. This is a problem for service managers
since they assume that the mountd manager parent exiting with status 0
means that the mountd manager PID is in the PID file. This behavior is
not possible with daemon(3). Drop it and do a service-manager-friendly
daemonization. The mountd manager signals successful daemonization by
sending the parent SIGUSR1. Note that we still do *not* perform the
double-fork trick for never acquiring a controlling terminal.

Currently, mountd also has problems regarding signal handling. Namely,
the handler mostly calls functions which are not async-signal-safe.
Also, the signal-block and signal-ignore masks are inherited from the
parent and are not reset. There is also the issue of the thread signal
mask as the helper thread spawned by g_thread_pool_new() inherits it
and passes it down to threads spawned with g_thread_pool_push().
Address these issues by only calling async-signal-safe functions in the
handler, resetting the signal-block and signal-ignore masks, and
blocking the signals before spawning the helper thread, respectively.
Also, block the signals when accessing global data accessed also in the
handler, so if the need arises, *some* not async-signal-safe functions
may be called.

Note that using pthread_sigmask() over sigprocmask() is a matter of
picking your poison, the use of the former assumes that GLib threads
are POSIX threads (sane) while the use of the latter assumes that it
only modifies the thread signal mask (sane).

Also, note that we now use select() in ipc_process_event() so a config
reload takes effect *immediately* rather than at the next IPC event.
We remove the KSMBD_SHOULD_RELOAD_CONFIG flag only on success.

Finally, note that the use of sigaction() with SA_NOCLDWAIT assumes
that the child still sends SIGCHLD to the parent, which is a Linuxism.
Alternatively, each sigwaitinfo() for SIGCHLD could be followed by a
waitpid() so as to prevent zombies.

With the above changes, an abort() in a GThreadPool thread (e.g. due to
GLib OoM) no longer results in a variety of use-after-frees. Also,
neither a segfault nor an initial pwddb/smbconf parse fail result in a
worker process restart.

As for other changes, don't zero `global_conf' with memset() since it
it is already zeroed and we do not depend on the padding being zero.

Signed-off-by: Atte Heikkilä <atteh.mailbox@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

configure.ac

@@ -89,6 +89,16 @@ AS_IF([test "x$with_systemdsystemunitdir" != xno], [
 
 AC_SUBST([systemdsystemunitdir])
 
+save_LIBS=$LIBS
+LIBS=
+AC_SEARCH_LIBS([pthread_sigmask], [pthread], [], [
+	AC_MSG_ERROR([pthread was not found.])
+])
+PTHREAD_LIBS=$LIBS
+LIBS=$save_LIBS
+
+AC_SUBST([PTHREAD_LIBS])
+
 PKG_CHECK_MODULES([GLIB], [glib-2.0 >= 2.44], [have_glib=yes], [have_glib=no])
 AS_IF([test "x$have_glib" != xyes], [
 	AC_MSG_ERROR([glib (libglib2.0-dev or glib2-devel) was not found.])

include/ipc.h

@@ -29,8 +29,6 @@ struct ksmbd_ipc_msg {
 #define KSMBD_IPC_MSG_PAYLOAD(m)				\
 	(void *)(((struct ksmbd_ipc_msg *)(m))->____payload)
 
-#define KSMBD_STATUS_IPC_FATAL_ERROR	11
-
 struct ksmbd_ipc_msg *ipc_msg_alloc(size_t sz);
 void ipc_msg_free(struct ksmbd_ipc_msg *msg);
 

meson.build

@@ -40,6 +40,10 @@ asn1_lib = []
 config_h_data = configuration_data()
 cc = meson.get_compiler('c')
 
+pthread_lib = cc.find_library(
+  'pthread',
+)
+
 if krb5_dep.found()
   config_h_data.set(
     'CONFIG_KRB5',

mountd/ipc.c

@@ -70,11 +70,6 @@ static int handle_generic_event(struct nl_cache_ops *unused,
 				struct genl_info *info,
 				void *arg)
 {
-	if (ksmbd_health_status & KSMBD_SHOULD_RELOAD_CONFIG) {
-		load_config(global_conf.pwddb, global_conf.smbconf);
-		ksmbd_health_status &= ~KSMBD_SHOULD_RELOAD_CONFIG;
-	}
-
 	if (!info->attrs[cmd->c_id])
 		return NL_SKIP;
 
@@ -208,14 +203,28 @@ static int ipc_ksmbd_starting_up(void)
 
 int ipc_process_event(void)
 {
-	int ret = 0;
+	fd_set rfds;
+	int sk_fd, ret;
 
-	ret = nl_recvmsgs_default(sk);
-	if (ret < 0) {
-		pr_err("nl_recv() returned error code %d: %s\n",
-		       ret, nl_geterror(ret));
-		return -KSMBD_STATUS_IPC_FATAL_ERROR;
+	FD_ZERO(&rfds);
+
+	sk_fd = nl_socket_get_fd(sk);
+	FD_SET(sk_fd, &rfds);
+
+	if (select(sk_fd + 1, &rfds, NULL, NULL, NULL) < 0) {
+		if (errno != EINTR) {
+			ret = -errno;
+			pr_err("Can't wait on netlink socket: %m\n");
+			return ret;
+		}
+		ret = 0;
+		return ret;
 	}
+
+	ret = nl_recvmsgs_default(sk);
+	if (ret < 0)
+		pr_err("nl_recv() failed, check dmesg, error: %s\n",
+		       nl_geterror(ret));
 	return ret;
 }