[LTP] [PATCH] verify_caps_exec: Respect TMP environment variable

[LTP] [PATCH] verify_caps_exec: Respect TMP environment variable

[Alistair Strachan]

The filecapstest.sh wrapper script already allows the /tmp directory to
be overridden with the TMP environment variable, however doing so has
no effect on verify_caps_exec because it creates its own version of this
fifo at a hardcoded location under /tmp.

To ensure the fifo is correctly removed by the wrapper script, alter
verify_caps_exec to respect the TMP environment variable and create a
fifo at the same location.

Signed-off-by: Alistair Strachan <astrachan@google.com>
---
 .../security/filecaps/verify_caps_exec.c      | 25 +++++++++++++++----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
index 2c5cc0b2a..ff0a4837b 100644
--- a/testcases/kernel/security/filecaps/verify_caps_exec.c
+++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
@@ -36,6 +36,7 @@
 #include <sys/wait.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include "config.h"
 #if HAVE_SYS_CAPABILITY_H
 #include <linux/types.h>
@@ -119,22 +120,36 @@ static int perms_test(void)
 	return ret;
 }
 
-#define FIFOFILE "/tmp/caps_fifo"
+static const char *get_caps_fifo(void)
+{
+	static char fifofile[PATH_MAX] = { 0, };
+
+	if (!fifofile[0]) {
+		const char *tmpdir = getenv("TMP");
+
+		if (!tmpdir)
+			tmpdir = "/tmp";
+		snprintf(fifofile, PATH_MAX, "%s/caps_fifo", tmpdir);
+	}
+
+	return fifofile;
+}
+
 static void create_fifo(void)
 {
 	int ret;
 
-	ret = mkfifo(FIFOFILE, S_IRWXU | S_IRWXG | S_IRWXO);
+	ret = mkfifo(get_caps_fifo(), S_IRWXU | S_IRWXG | S_IRWXO);
 	if (ret == -1 && errno != EEXIST)
 		tst_brkm(TFAIL | TERRNO, NULL, "failed creating %s\n",
-			 FIFOFILE);
+			 get_caps_fifo());
 }
 
 static void write_to_fifo(const char *buf)
 {
 	int fd;
 
-	fd = open(FIFOFILE, O_WRONLY);
+	fd = open(get_caps_fifo(), O_WRONLY);
 	write(fd, buf, strlen(buf));
 	close(fd);
 }
@@ -144,7 +159,7 @@ static void read_from_fifo(char *buf)
 	int fd;
 
 	memset(buf, 0, 200);
-	fd = open(FIFOFILE, O_RDONLY);
+	fd = open(get_caps_fifo(), O_RDONLY);
 	if (fd < 0)
 		tst_brkm(TFAIL | TERRNO, NULL, "Failed opening fifo\n");
 	read(fd, buf, 199);

[LTP] [PATCH] verify_caps_exec: Respect TMP environment variable

[Jan Stancek]

----- Original Message -----
> The filecapstest.sh wrapper script already allows the /tmp directory to
> be overridden with the TMP environment variable, however doing so has
> no effect on verify_caps_exec because it creates its own version of this
> fifo at a hardcoded location under /tmp.
> 
> To ensure the fifo is correctly removed by the wrapper script, alter
> verify_caps_exec to respect the TMP environment variable and create a
> fifo at the same location.

Hi,

this doesn't seem to be enough, print_caps.c is also hardcoding /tmp.
Other than rewriting the test, I'm thinking simplest way to fix this is:

1. export FIFOFILE in wrapper script and then use that env. variable
in all *.c tests.

2. change wrapper script to use $TMPDIR if available
(that is the value exported by runltp)

What do you think?

Regards,
Jan

[LTP] [PATCH] filecaps: Respect TMPDIR environment variable

[Alistair Strachan]

The filecapstest.sh wrapper script already allowed the /tmp directory to
be overridden with the TMP environment variable, however doing so had
no effect on verify_caps_exec because it created its own version of this
fifo at a hardcoded location under /tmp.

Change the wrapper script to check for TMPDIR instead of TMP, to match
the value exported by runltp. Export FIFOFILE, to be used by the test
binaries invoked by the script.

Change the print_caps and verify_caps_exec to read FIFOFILE from the
environment (if it exists). Otherwise, TMPDIR will be read from the
environment and used to construct the path to the caps_fifo file.

Signed-off-by: Alistair Strachan <astrachan@google.com>
---
 .../kernel/security/filecaps/filecapstest.sh  |  5 ++-
 .../kernel/security/filecaps/print_caps.c     | 29 +++++++++++++++--
 .../security/filecaps/verify_caps_exec.c      | 32 ++++++++++++++++---
 3 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/testcases/kernel/security/filecaps/filecapstest.sh b/testcases/kernel/security/filecaps/filecapstest.sh
index 213b095bb..9bb5702df 100755
--- a/testcases/kernel/security/filecaps/filecapstest.sh
+++ b/testcases/kernel/security/filecaps/filecapstest.sh
@@ -22,9 +22,8 @@
 echo "Running in:"
 #rm -f print_caps
 #cp $LTPROOT/testcases/bin/print_caps .
-#FIFOFILE="$LTPROOT/testcases/bin/caps_fifo"
-TMP=${TMP:=/tmp}
-FIFOFILE="$TMP/caps_fifo"
+FIFOFILE="${TMPDIR:=/tmp}/caps_fifo"
+export FIFOFILE
 rm -f $FIFOFILE
 mkfifo $FIFOFILE
 chmod 777 $FIFOFILE
diff --git a/testcases/kernel/security/filecaps/print_caps.c b/testcases/kernel/security/filecaps/print_caps.c
index ee7a5f580..2479c4590 100644
--- a/testcases/kernel/security/filecaps/print_caps.c
+++ b/testcases/kernel/security/filecaps/print_caps.c
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <limits.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -37,7 +38,31 @@
 #include <sys/capability.h>
 #endif
 
-#define FIFOFILE "/tmp/caps_fifo"
+#ifdef HAVE_LIBCAP
+
+static const char *get_caps_fifo(void)
+{
+	static char fifofile[PATH_MAX] = { 0, };
+
+	if (!fifofile[0]) {
+		const char *fifofile_ = getenv("FIFOFILE");
+
+		if (!fifofile_) {
+			const char *tmpdir = getenv("TMPDIR");
+
+			if (!tmpdir)
+				tmpdir = "/tmp";
+			snprintf(fifofile, PATH_MAX, "%s/caps_fifo", tmpdir);
+		} else {
+			strncpy(fifofile, fifofile_, PATH_MAX);
+			fifofile[PATH_MAX - 1] = 0;
+		}
+	}
+
+	return fifofile;
+}
+
+#endif
 
 int main(int argc, char *argv[])
 {
@@ -55,7 +80,7 @@ int main(int argc, char *argv[])
 		exit(1);
 	}
 
-	fd = open(FIFOFILE, O_WRONLY);
+	fd = open(get_caps_fifo(), O_WRONLY);
 	if (!fd) {
 		perror("print_caps: open fifo");
 		exit(2);
diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
index 7183d8b4a..fa2f0659c 100644
--- a/testcases/kernel/security/filecaps/verify_caps_exec.c
+++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
@@ -36,6 +36,7 @@
 #include <sys/wait.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include "config.h"
 #if HAVE_SYS_CAPABILITY_H
 #include <linux/types.h>
@@ -119,22 +120,43 @@ static int perms_test(void)
 	return ret;
 }
 
-#define FIFOFILE "/tmp/caps_fifo"
+static const char *get_caps_fifo(void)
+{
+	static char fifofile[PATH_MAX] = { 0, };
+
+	if (!fifofile[0]) {
+		const char *fifofile_ = getenv("FIFOFILE");
+
+		if (!fifofile_) {
+			const char *tmpdir = getenv("TMPDIR");
+
+			if (!tmpdir)
+				tmpdir = "/tmp";
+			snprintf(fifofile, PATH_MAX, "%s/caps_fifo", tmpdir);
+		} else {
+			strncpy(fifofile, fifofile_, PATH_MAX);
+			fifofile[PATH_MAX - 1] = 0;
+		}
+	}
+
+	return fifofile;
+}
+
 static void create_fifo(void)
 {
 	int ret;
 
-	ret = mkfifo(FIFOFILE, S_IRWXU | S_IRWXG | S_IRWXO);
+	ret = mkfifo(get_caps_fifo(), S_IRWXU | S_IRWXG | S_IRWXO);
 	if (ret == -1 && errno != EEXIST)
 		tst_brkm(TFAIL | TERRNO, NULL, "failed creating %s\n",
-			 FIFOFILE);
+			 get_caps_fifo());
 }
 
 static void write_to_fifo(const char *buf)
 {
 	int fd;
 
-	fd = open(FIFOFILE, O_WRONLY);
+	fd = open(get_caps_fifo(), O_WRONLY);
 	write(fd, buf, strlen(buf));
 	close(fd);
 }
@@ -144,7 +166,7 @@ static void read_from_fifo(char *buf)
 	int fd;
 
 	memset(buf, 0, 200);
-	fd = open(FIFOFILE, O_RDONLY);
+	fd = open(get_caps_fifo(), O_RDONLY);
 	if (fd < 0)
 		tst_brkm(TFAIL | TERRNO, NULL, "Failed opening fifo\n");
 	read(fd, buf, 199);

[LTP] [PATCH] filecaps: Respect TMPDIR environment variable

[Jan Stancek]

----- Original Message -----
> The filecapstest.sh wrapper script already allowed the /tmp directory to
> be overridden with the TMP environment variable, however doing so had
> no effect on verify_caps_exec because it created its own version of this
> fifo at a hardcoded location under /tmp.
> 
> Change the wrapper script to check for TMPDIR instead of TMP, to match
> the value exported by runltp. Export FIFOFILE, to be used by the test
> binaries invoked by the script.
> 
> Change the print_caps and verify_caps_exec to read FIFOFILE from the
> environment (if it exists). Otherwise, TMPDIR will be read from the
> environment and used to construct the path to the caps_fifo file.
> 
> Signed-off-by: Alistair Strachan <astrachan@google.com>

I moved get_caps_fifo() to common header, changed it to use
getenv() ptr directly when possible and pushed.

Thanks,
Jan

[LTP] [PATCH] verify_caps_exec: Respect TMP environment variable

[Petr Vorel]

Hi Alistair, Jan,

> ----- Original Message -----
> > The filecapstest.sh wrapper script already allows the /tmp directory to
> > be overridden with the TMP environment variable, however doing so has
> > no effect on verify_caps_exec because it creates its own version of this
> > fifo at a hardcoded location under /tmp.

> > To ensure the fifo is correctly removed by the wrapper script, alter
> > verify_caps_exec to respect the TMP environment variable and create a
> > fifo at the same location.

> Hi,

> this doesn't seem to be enough, print_caps.c is also hardcoding /tmp.
> Other than rewriting the test, I'm thinking simplest way to fix this is:

> 1. export FIFOFILE in wrapper script and then use that env. variable
> in all *.c tests.

> 2. change wrapper script to use $TMPDIR if available
> (that is the value exported by runltp)

> What do you think?
I guess this issue has been solved by 
cf8bd0327 ("filecaps: Respect TMPDIR environment variable"), thus closing it in
patchwork [1]

> Regards,
> Jan

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/patch/934131/