From 24806f6f612501aa92a9449bf73dde34f5624627 Mon Sep 17 00:00:00 2001
From: Georgia Garcia <georgia.garcia@canonical.com>
Date: Mon, 21 Aug 2023 12:04:51 -0300
Subject: [PATCH] tests: fix userns setns opening pipe order
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

setns tests part of the userns could fail if the parent process opened
the child pipe to write it was done before the child opened the pipe
with read permissions.

From the fifo(7) man page:

A process can open a FIFO in nonblocking mode.  In this case, opening
for read‐only succeeds even if no one has opened on the write side yet
and opening for write‐only fails with ENXIO (no such device or
address) unless the other end has already been opened.

Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
---
 tests/regression/apparmor/userns.c       | 10 +++++++++-
 tests/regression/apparmor/userns.h       | 20 +++++++++++++++-----
 tests/regression/apparmor/userns_setns.c | 10 +++++++++-
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/tests/regression/apparmor/userns.c b/tests/regression/apparmor/userns.c
index bc2beb596..b10144752 100644
--- a/tests/regression/apparmor/userns.c
+++ b/tests/regression/apparmor/userns.c
@@ -58,6 +58,7 @@ int userns_setns(char *client, char *pipename)
 {
 	int userns, exit_status, ret;
 	char *parentpipe = NULL, *childpipe = NULL;
+	int parentpipefd;
 
 	if (get_pipes(pipename, &parentpipe, &childpipe) == -1) {
 		fprintf(stderr, "FAIL - failed to allocate pipes\n");
@@ -81,7 +82,14 @@ int userns_setns(char *client, char *pipename)
 		goto out;
 	}
 
-	if (read_from_pipe(parentpipe) == -1) { // wait for child to unshare
+	parentpipefd = open_read_pipe(parentpipe);
+	if (parentpipefd == -1) {
+		fprintf(stderr, "FAIL - couldn't open parent pipe\n");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+
+	if (read_from_pipe(parentpipefd) == -1) { // wait for child to unshare
 		fprintf(stderr, "FAIL - parent could not read from pipe\n");
 		ret = EXIT_FAILURE;
 		goto out;
diff --git a/tests/regression/apparmor/userns.h b/tests/regression/apparmor/userns.h
index d31cda51c..b3bb699f3 100644
--- a/tests/regression/apparmor/userns.h
+++ b/tests/regression/apparmor/userns.h
@@ -15,16 +15,26 @@ int get_pipes(const char *pipename, char **parentpipe, char **childpipe)
 	return 0;
 }
 
-int read_from_pipe(char *pipename)
+int open_read_pipe(char *pipename)
 {
-	int fd, ret;
+	int fd;
+	fd = open(pipename, O_RDONLY | O_NONBLOCK);
+	if (fd == -1) {
+		perror("FAIL - open read pipe");
+		return EXIT_FAILURE;
+	}
+	return fd;
+}
+
+int read_from_pipe(int fd)
+{
+	int ret;
 	char buf;
 	fd_set set;
 	struct timeval timeout;
 
-	fd = open(pipename, O_RDONLY | O_NONBLOCK);
 	if (fd == -1) {
-		perror("FAIL - open read pipe");
+		fprintf(stderr, "FAIL - invalid read fd\n");
 		return EXIT_FAILURE;
 	}
 
@@ -59,7 +69,7 @@ int write_to_pipe(char *pipename)
 
 	fd = open(pipename, O_WRONLY | O_NONBLOCK);
 	if (fd == -1) {
-		perror("FAIL - open write pipe");
+		fprintf(stderr, "FAIL - open write pipe %s - %m\n", pipename);
 		return EXIT_FAILURE;
 	}
 	close(fd);
diff --git a/tests/regression/apparmor/userns_setns.c b/tests/regression/apparmor/userns_setns.c
index 64815e9b2..f85ed4071 100644
--- a/tests/regression/apparmor/userns_setns.c
+++ b/tests/regression/apparmor/userns_setns.c
@@ -13,6 +13,7 @@ int main(int argc, char *argv[])
 	int ret;
 	char *pipename = "/tmp/userns_pipe";
 	char *parentpipe = NULL, *childpipe = NULL;
+	int childpipefd;
 
 	if (argc > 1)
 		pipename = argv[1];
@@ -26,6 +27,13 @@ int main(int argc, char *argv[])
 	if (mkfifo(childpipe, 0666) == -1)
 		perror("FAIL - setns child mkfifo");
 
+	childpipefd = open_read_pipe(childpipe);
+	if (childpipefd == -1) {
+		fprintf(stderr, "FAIL - couldn't open child pipe\n");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+
 	if (unshare(CLONE_NEWUSER) == -1) {
 		perror("FAIL - unshare");
 		ret = EXIT_FAILURE;
@@ -37,7 +45,7 @@ int main(int argc, char *argv[])
 		ret = EXIT_FAILURE;
 		goto out;
 	}
-	if (read_from_pipe(childpipe) == -1) { // wait for parent tell child can finish
+	if (read_from_pipe(childpipefd) == -1) { // wait for parent tell child can finish
 		fprintf(stderr, "FAIL - child could not read from pipe\n");
 		ret = EXIT_FAILURE;
 		goto out;