[PATCH 1/6] test: Add missing test-statx test case

[PATCH 1/6] test: Add missing test-statx test case

[Richard Purdie]

From: Philip Lorenz <philip@bithub.de>

Adding this test case was erroneously omitted in
7c722296879906fe093e1e7c4b7537e150d492cd.

Signed-off-by: Philip Lorenz <philip@bithub.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 test/test-statx.c  | 20 ++++++++++++++++++++
 test/test-statx.sh |  6 ++++++
 2 files changed, 26 insertions(+)
 create mode 100644 test/test-statx.c
 create mode 100755 test/test-statx.sh

diff --git a/test/test-statx.c b/test/test-statx.c
new file mode 100644
index 0000000..06d86af
--- /dev/null
+++ b/test/test-statx.c
@@ -0,0 +1,20 @@
+/*
+ * Test that passing NULL to a parameter marked as nonnull works correctly
+ * SPDX-License-Identifier: LGPL-2.1-only
+ *
+ */
+#define _GNU_SOURCE
+
+#include <sys/stat.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+// Passing a null pointer is the test scenario
+#pragma GCC diagnostic ignored "-Wnonnull"
+
+int main(void) {
+    if (statx(0, NULL, 0, 0, NULL) != -1) {
+        return 1;
+    }
+    return 0;
+}
diff --git a/test/test-statx.sh b/test/test-statx.sh
new file mode 100755
index 0000000..77d0302
--- /dev/null
+++ b/test/test-statx.sh
@@ -0,0 +1,6 @@
+#!/bin/bash
+#
+# SPDX-License-Identifier: LGPL-2.1-only
+#
+
+exec ./test/test-statx
-- 
2.32.0

[PATCH 2/6] test-openat: Consider device as well as inode number

[Richard Purdie]

From: Mike Crowe <mac@mcrowe.com>

It just so happens that my /home/mac and /home directories have the same
inode number but on different filesystems.

This means that test-openat fails with "Recursion failed!" even when run
without pseudo.

Let's consider both the device number and the inode number before
assuming that we've found the same directory again.

Signed-off-by: Mike Crowe <mac@mcrowe.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 test/test-openat.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/test/test-openat.c b/test/test-openat.c
index b710285..df6655a 100644
--- a/test/test-openat.c
+++ b/test/test-openat.c
@@ -25,11 +25,13 @@ int main () {
     int fd, dir_fd;
     struct stat st;
     ino_t ino;
+    dev_t dev;
     char *path;
 
     fd = openat(AT_FDCWD, ".", O_DIRECTORY, 0);
     fstat(fd, &st);
     ino = st.st_ino;
+    dev = st.st_dev;
 
     while (1) {
         path = path_of(fd);
@@ -37,7 +39,7 @@ int main () {
 
         dir_fd = openat(fd, "../", O_DIRECTORY, 0);
         fstat(dir_fd, &st);
-        if (st.st_ino == ino) {
+        if (st.st_ino == ino && st.st_dev == dev) {
             if (strcmp(path, "/") == 0) {
                 //puts("Reached top of tree");
                 return 0;
@@ -49,6 +51,7 @@ int main () {
 
         free (path);
         ino = st.st_ino;
+        dev = st.st_dev;
         fd = dir_fd;
     }
     return 0;
-- 
2.32.0

[PATCH 3/6] pseudo_client: Do not pass null argument to pseudo_diag()

[Richard Purdie]

From: Damian Wrobel <dwrobel@ertelnet.rybnik.pl>

Fixes the following warning:

 pseudo_client.c: In function ‘pseudo_root_path’:
 pseudo_client.c:848:17: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
   848 |                 pseudo_diag("couldn't allocate absolute path for '%s'.\n",
       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   849 |                         path);
       |                         ~~~~~

Signed-off-by: Damian Wrobel <dwrobel@ertelnet.rybnik.pl>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 pseudo_client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pseudo_client.c b/pseudo_client.c
index 579db33..2583bca 100644
--- a/pseudo_client.c
+++ b/pseudo_client.c
@@ -846,7 +846,7 @@ pseudo_root_path(const char *func, int line, int dirfd, const char *path, int le
 	pseudo_magic();
 	if (!rc) {
 		pseudo_diag("couldn't allocate absolute path for '%s'.\n",
-			path);
+			path ? path : "null");
 	}
 	pseudo_debug(PDBGF_CHROOT, "root_path [%s, %d]: '%s' from '%s'\n",
 		func, line,
-- 
2.32.0

[PATCH 4/6] ports/linux/guts: Add close_range wrapper for glibc 2.34

[Richard Purdie]

glibc 2.34 adds a close_range() function call. This one is straight forward
as it allows ENOSYS to be returned and the caller has to handle it so
lets do that.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 ports/linux/guts/close_range.c | 20 ++++++++++++++++++++
 ports/linux/wrapfuncs.in       |  1 +
 2 files changed, 21 insertions(+)
 create mode 100644 ports/linux/guts/close_range.c

diff --git a/ports/linux/guts/close_range.c b/ports/linux/guts/close_range.c
new file mode 100644
index 0000000..4bd2fe1
--- /dev/null
+++ b/ports/linux/guts/close_range.c
@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2021 Richard Purdie
+ *
+ * SPDX-License-Identifier: LGPL-2.1-only
+ *
+ * int close_range(unsigned int lowfd, unsigned int maxfd, int flags)
+ *      int rc = -1;
+ */
+
+        (void) lowfd;
+        (void) maxfd;
+        (void) flags;
+        /* for now pretend the kernel doesn't support it regardless 
+           which users are supposed to be able to handle */
+        errno = ENOSYS;
+        rc = -1;
+
+/*      return rc;
+ * }
+ */
diff --git a/ports/linux/wrapfuncs.in b/ports/linux/wrapfuncs.in
index 3824fd8..e978367 100644
--- a/ports/linux/wrapfuncs.in
+++ b/ports/linux/wrapfuncs.in
@@ -61,3 +61,4 @@ int capset(cap_user_header_t hdrp, const cap_user_data_t datap); /* real_func=ps
 long syscall(long nr, ...); /* hand_wrapped=1 */
 int renameat2(int olddirfd, const char *oldpath, int newdirfd, const char *newpath, unsigned int flags); /* flags=AT_SYMLINK_NOFOLLOW */
 int prctl(int option, ...); /* hand_wrapped=1 */
+int close_range(unsigned int lowfd, unsigned int maxfd, int flags);
-- 
2.32.0

[PATCH 5/6] pseudo_client: Make msg static in pseudo_op_client

[Richard Purdie]

The address of msg is returned by the function in the OP_CHROOT case.
Whilst the way it is used means it is just a way of saying true/false,
the compiler doesn't know this and can warn and this isn't really
allowed.

The other pseudo funcitons in this area already use a static msg so
make this function match the others. There wouldn't be re-enterancy
in this context.

Need to be careful about clearing the data upon each function entry
and not just once at init.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 pseudo_client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pseudo_client.c b/pseudo_client.c
index 2583bca..546bdb1 100644
--- a/pseudo_client.c
+++ b/pseudo_client.c
@@ -1589,7 +1589,8 @@ int pseudo_client_ignore_path(const char *path) {
 pseudo_msg_t *
 pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path, const PSEUDO_STATBUF *buf, ...) {
 	pseudo_msg_t *result = 0;
-	pseudo_msg_t msg = { .type = PSEUDO_MSG_OP };
+	static pseudo_msg_t msg;
+	msg = (pseudo_msg_t) { .type = PSEUDO_MSG_OP };
 	size_t pathlen = -1;
 	int do_request = 0;
 	char *path_extra_1 = 0;
-- 
2.32.0

[PATCH 6/6] ports/linux/guts: Add closefrom support for glibc 2.34

[Richard Purdie]

glibc 2.34 adds a closefrom() function call to close a range of file
descriptors. This one is problematic for us since pseudo can have its
own fds open in the close range.

To handle this we add a specific client side op, OP_CLOSEFROM, similar
to OP_CLOSE which closes the fds in the range which aren't pseudo fds.

This means manually closing some of the fds ourselves and then modifying
the call to closefrom for the rest. Not pretty but I'm struggling to see
a better way. It does mean msg/result is used in a new case to let the
caller know which fds to close as the range needs to change. This is
allowed after the previous static change.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 enums/op.in                  |  1 +
 ports/linux/guts/closefrom.c | 15 +++++++++++
 ports/linux/wrapfuncs.in     |  1 +
 pseudo_client.c              | 48 ++++++++++++++++++++++++++++++++++--
 4 files changed, 63 insertions(+), 2 deletions(-)
 create mode 100644 ports/linux/guts/closefrom.c

diff --git a/enums/op.in b/enums/op.in
index 61ee666..5b5e21b 100644
--- a/enums/op.in
+++ b/enums/op.in
@@ -27,3 +27,4 @@ remove-xattr, 1
 set-xattr, 0
 create-xattr, 1
 replace-xattr, 1
+closefrom, 0
diff --git a/ports/linux/guts/closefrom.c b/ports/linux/guts/closefrom.c
new file mode 100644
index 0000000..35df71e
--- /dev/null
+++ b/ports/linux/guts/closefrom.c
@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2021 Richard Purdie
+ *
+ * SPDX-License-Identifier: LGPL-2.1-only
+ *
+ * void closefrom(int fd)
+ */
+ 	pseudo_msg_t *msg;
+	/* this cleans up internal tables, and shouldn't make it to the server. Avoids pseudo's internal fds */
+	msg = pseudo_client_op(OP_CLOSEFROM, 0, fd, -1, 0, 0);
+	real_closefrom(msg->fd);
+
+/*      return;
+ * }
+ */
diff --git a/ports/linux/wrapfuncs.in b/ports/linux/wrapfuncs.in
index e978367..793f5d8 100644
--- a/ports/linux/wrapfuncs.in
+++ b/ports/linux/wrapfuncs.in
@@ -62,3 +62,4 @@ long syscall(long nr, ...); /* hand_wrapped=1 */
 int renameat2(int olddirfd, const char *oldpath, int newdirfd, const char *newpath, unsigned int flags); /* flags=AT_SYMLINK_NOFOLLOW */
 int prctl(int option, ...); /* hand_wrapped=1 */
 int close_range(unsigned int lowfd, unsigned int maxfd, int flags);
+void closefrom(int fd);
diff --git a/pseudo_client.c b/pseudo_client.c
index 546bdb1..a03d6b1 100644
--- a/pseudo_client.c
+++ b/pseudo_client.c
@@ -983,6 +983,23 @@ pseudo_client_close(int fd) {
 	}
 }
 
+static void
+pseudo_client_closefrom(int fd) {
+	int i;
+	if (fd < 0 || fd >= nfds)
+		return;
+
+	for (i = fd; i < nfds; ++i) {
+		free(fd_paths[i]);
+		fd_paths[i] = 0;
+
+		if (i < linked_nfds) {
+			free(linked_fd_paths[i]);
+			linked_fd_paths[i] = 0;
+		}
+	}
+}
+
 /* spawn server */
 static int
 client_spawn_server(void) {
@@ -1600,6 +1617,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 	static char *alloced_path = 0;
 	static size_t alloced_len = 0;
 	int strip_slash;
+	int startfd, i;
 
 #ifdef PSEUDO_PROFILING
 	struct timeval tv1_op, tv2_op;
@@ -1627,7 +1645,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 		}
 	}
 
-	if (op != OP_CHROOT && op != OP_CHDIR && op != OP_CLOSE && op != OP_DUP
+	if (op != OP_CHROOT && op != OP_CHDIR && op != OP_CLOSE && op != OP_CLOSEFROM && op != OP_DUP
 			&& pseudo_client_ignore_path_chroot(path, 0)) {
 		if (op == OP_OPEN) {
 			/* Sanitise the path to have no trailing slash as this is convention in the database */
@@ -1900,6 +1918,32 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 	case OP_EXEC:
 		do_request = pseudo_client_logging;
 		break;
+	case OP_CLOSEFROM:
+		/* no request needed */
+		startfd = fd;
+		if (pseudo_util_debug_fd > startfd)
+			startfd = pseudo_util_debug_fd + 1;
+		if (pseudo_localstate_dir_fd > startfd)
+			startfd = pseudo_localstate_dir_fd + 1;
+		if (pseudo_pwd_fd > startfd)
+			startfd = pseudo_pwd_fd + 1;
+		if (pseudo_grp_fd > startfd)
+			startfd = pseudo_grp_fd + 1;
+		if (connect_fd > startfd)
+			startfd = connect_fd + 1;
+		for (i = fd; i < startfd; ++i) {
+			if (i == pseudo_util_debug_fd || i == pseudo_localstate_dir_fd || i == pseudo_pwd_fd ||
+					i == pseudo_grp_fd || i == connect_fd)
+				continue;
+			pseudo_client_close(i);
+			close(i);
+		}
+		pseudo_client_closefrom(startfd);
+		/* tell the caller to close from startfd instead of fd */
+		result = &msg;
+		msg.fd = startfd;
+		do_request = 0;
+		break;
 	case OP_CLOSE:
 		/* no request needed */
 		if (fd >= 0) {
@@ -1982,7 +2026,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 		break;
 	}
 	/* result can only be set when PSEUDO_XATTRDB resulted in a
-	 * successful store to or read from the local database.
+	 * successful store to or read from the local database or for OP_CLOSEFROM.
 	 */
 	if (do_request && !result) {
 #ifdef PSEUDO_PROFILING
-- 
2.32.0

Re: [PATCH 5/6] pseudo_client: Make msg static in pseudo_op_client

[Seebs]

On Thu,  9 Sep 2021 16:36:13 +0100
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> -	pseudo_msg_t msg = { .type = PSEUDO_MSG_OP };
> +	static pseudo_msg_t msg;
> +	msg = (pseudo_msg_t) { .type = PSEUDO_MSG_OP };

Looks good to me.

I'm honestly sort of stunned that the return of the local variable's
address caused a problem, even though it's theoretically undefined,
because it's only ever checked against is-null, but I'm also sort of
stunned that I got away with it for that long.

-s

Re: [PATCH 3/6] pseudo_client: Do not pass null argument to pseudo_diag()

[Seebs]

On Thu,  9 Sep 2021 16:36:11 +0100
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

>  pseudo_client.c:848:17: warning: ‘%s’ directive argument is null
> [-Wformat-overflow=] 848 |                 pseudo_diag("couldn't
> allocate absolute path for '%s'.\n",

I'm sort of curious as to whether pseudo_root_path is ever actually
called on a null path, but eh, checks are cheap.

-s

Re: [PATCH 2/6] test-openat: Consider device as well as inode number

[Seebs]

On Thu,  9 Sep 2021 16:36:10 +0100
Richard Purdie <richard.purdie@linuxfoundation.org> wrote:

> It just so happens that my /home/mac and /home directories have the
> same inode number but on different filesystems.

Nice catch!

-s