Fix FIFO races

clish/shell.h

@@ -132,6 +132,8 @@ clish_var_t *clish_shell_find_var(clish_shell_t *instance, const char *name);
 char *clish_shell_expand_var(const char *name, clish_context_t *context);
 char *clish_shell_expand_var_ex(const char *name, clish_context_t *context, clish_shell_expand_e flags);
 char *clish_shell_expand(const char *str, clish_shell_var_e vtype, clish_context_t *context);
+char * clish_shell_mkfifo(clish_shell_t * instance, char *name, size_t n);
+int clish_shell_rmfifo(clish_shell_t * instance, const char *name);
 
 /*-----------------
  * attributes
@@ -171,7 +173,6 @@ void clish_shell__set_startup_view(clish_shell_t * instance, const char * viewna
 void clish_shell__set_startup_viewid(clish_shell_t * instance, const char * viewid);
 void clish_shell__set_default_shebang(clish_shell_t * instance, const char * shebang);
 const char * clish_shell__get_default_shebang(const clish_shell_t * instance);
-const char * clish_shell__get_fifo(clish_shell_t * instance);
 void clish_shell__set_interactive(clish_shell_t * instance, bool_t interactive);
 bool_t clish_shell__get_interactive(const clish_shell_t * instance);
 bool_t clish_shell__get_utf8(const clish_shell_t * instance);

clish/shell/private.h

@@ -81,7 +81,7 @@ struct clish_shell_s {
 	konf_client_t *client;
 	char *lockfile;
 	char *default_shebang;
-	char *fifo_name; /* The name of temporary fifo file. */
+	char *fifo_temp; /* The template of temporary FIFO file name */
 	struct passwd *user; /* Current user information */
 
 	/* Boolean flags */

clish/shell/shell_execute.c

@@ -397,30 +397,28 @@ CLISH_HOOK_LOG(clish_shell_exec_log)
 }
 
 /*----------------------------------------------------------- */
-const char *clish_shell__get_fifo(clish_shell_t * this)
+char *clish_shell_mkfifo(clish_shell_t * this, char *name, size_t n)
 {
-	char *name;
 	int res;
 
-	if (this->fifo_name) {
-		if (0 == access(this->fifo_name, R_OK | W_OK))
-			return this->fifo_name;
-		unlink(this->fifo_name);
-		lub_string_free(this->fifo_name);
-		this->fifo_name = NULL;
-	}
-
+	if (n < 1) /* Buffer too small */
+		return NULL;
 	do {
-		char template[] = "/tmp/klish.fifo.XXXXXX";
-		name = mktemp(template);
+		strncpy(name, this->fifo_temp, n);
+		name[n - 1] = '\0';
+		mktemp(name);
 		if (name[0] == '\0')
 			return NULL;
 		res = mkfifo(name, 0600);
-		if (res == 0)
-			this->fifo_name = lub_string_dup(name);
 	} while ((res < 0) && (EEXIST == errno));
 
-	return this->fifo_name;
+	return name;
+}
+
+/*----------------------------------------------------------- */
+int clish_shell_rmfifo(clish_shell_t * this, const char *name)
+{
+	return unlink(name);
 }
 
 /*-------------------------------------------------------- */

clish/shell/shell_new.c

@@ -8,6 +8,7 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <syslog.h>
+#include <limits.h>
 
 #include "lub/string.h"
 #include "lub/db.h"
@@ -20,6 +21,7 @@ static void clish_shell_init(clish_shell_t * this,
 {
 	clish_ptype_t *tmp_ptype = NULL;
 	int i;
+	char template[PATH_MAX];
 
 	/* initialise the tree of views */
 	lub_bintree_init(&this->view_tree,
@@ -69,7 +71,6 @@ static void clish_shell_init(clish_shell_t * this,
 	this->client = NULL;
 	this->lockfile = lub_string_dup(CLISH_LOCK_PATH);
 	this->default_shebang = lub_string_dup("/bin/sh");
-	this->fifo_name = NULL;
 	this->interactive = BOOL_TRUE; /* The interactive shell by default. */
 	this->log = BOOL_FALSE; /* Disable logging by default */
 	this->log_facility = LOG_LOCAL0; /* LOCAL0 for compatibility */
@@ -77,6 +78,12 @@ static void clish_shell_init(clish_shell_t * this,
 	this->user = lub_db_getpwuid(getuid()); /* Get user information */
 	this->default_plugin = BOOL_TRUE; /* Load default plugin by default */
 
+	/* Create template (string) for FIFO name generation */
+	snprintf(template, sizeof(template),
+		"%s/klish.fifo.%u.XXXXXX", "/tmp", getpid());
+	template[sizeof(template) - 1] = '\0';
+	this->fifo_temp = lub_string_dup(template);
+
 	/* Create internal ptypes and params */
 	/* Args */
 	tmp_ptype = clish_shell_find_create_ptype(this,
@@ -182,10 +189,8 @@ static void clish_shell_fini(clish_shell_t *this)
 	lub_string_free(this->lockfile);
 	lub_string_free(this->default_shebang);
 	free(this->user);
-	if (this->fifo_name) {
-		unlink(this->fifo_name);
-		lub_string_free(this->fifo_name);
-	}
+	if (this->fifo_temp)
+		lub_string_free(this->fifo_temp);
 }
 
 /*-------------------------------------------------------- */

plugins/clish/sym_script.c

@@ -19,6 +19,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
+#include <limits.h>
 
 /*--------------------------------------------------------- */
 CLISH_PLUGIN_OSYM(clish_script)
@@ -28,7 +29,7 @@ CLISH_PLUGIN_OSYM(clish_script)
 	const char *shebang = NULL;
 	pid_t cpid = -1;
 	int res;
-	const char *fifo_name;
+	char fifo_name[PATH_MAX];
 	FILE *wpipe;
 	char *command = NULL;
 
@@ -48,9 +49,8 @@ CLISH_PLUGIN_OSYM(clish_script)
 	fprintf(stderr, "SCRIPT: %s\n", script);
 #endif /* DEBUG */
 
-	/* Get FIFO */
-	fifo_name = clish_shell__get_fifo(this);
-	if (!fifo_name) {
+	/* Create FIFO */
+	if (! clish_shell_mkfifo(this, fifo_name, sizeof(fifo_name))) {
 		fprintf(stderr, "Error: Can't create temporary FIFO.\n"
 			"Error: The ACTION will be not executed.\n");
 		return -1;
@@ -61,6 +61,7 @@ CLISH_PLUGIN_OSYM(clish_script)
 	if (cpid == -1) {
 		fprintf(stderr, "Error: Can't fork the write process.\n"
 			"Error: The ACTION will be not executed.\n");
+		clish_shell_rmfifo(this, fifo_name);
 		return -1;
 	}
 
@@ -86,7 +87,9 @@ CLISH_PLUGIN_OSYM(clish_script)
 	kill(cpid, SIGTERM);
 	waitpid(cpid, NULL, 0);
 
+	/* Clean up */
 	lub_string_free(command);
+	clish_shell_rmfifo(this, fifo_name);
 
 #ifdef DEBUG
 	fprintf(stderr, "RETCODE: %d\n", WEXITSTATUS(res));