[PATCH] passt-repair: Add directory watch

[PATCH] passt-repair: Add directory watch

[Stefano Brivio]

It might not be feasible for users to start passt-repair after passt
is started, on a migration target, but before the migration process
starts.

For instance, with libvirt, the guest domain (and, hence, passt) is
started on the target as part of the migration process. At least for
the moment being, there's no hook a libvirt user (including KubeVirt)
can use to start passt-repair before the migration starts.

Add a directory watch using inotify: if PATH is a directory, instead
of connecting to it, we'll watch for a .repair socket file to appear
in it, and then attempt to connect to that socket.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 contrib/selinux/passt-repair.te | 16 +++----
 passt-repair.1                  |  6 ++-
 passt-repair.c                  | 82 ++++++++++++++++++++++++++++++---
 3 files changed, 88 insertions(+), 16 deletions(-)

diff --git a/contrib/selinux/passt-repair.te b/contrib/selinux/passt-repair.te
index f171be6..7157dfb 100644
--- a/contrib/selinux/passt-repair.te
+++ b/contrib/selinux/passt-repair.te
@@ -61,11 +61,11 @@ allow passt_repair_t unconfined_t:unix_stream_socket { connectto read write };
 allow passt_repair_t passt_t:unix_stream_socket { connectto read write };
 allow passt_repair_t user_tmp_t:unix_stream_socket { connectto read write };
 
-allow passt_repair_t user_tmp_t:dir search;
+allow passt_repair_t user_tmp_t:dir { getattr read search watch };
 
-allow passt_repair_t unconfined_t:sock_file { read write };
-allow passt_repair_t passt_t:sock_file { read write };
-allow passt_repair_t user_tmp_t:sock_file { read write };
+allow passt_repair_t unconfined_t:sock_file { getattr read write };
+allow passt_repair_t passt_t:sock_file { getattr read write };
+allow passt_repair_t user_tmp_t:sock_file { getattr read write };
 
 allow passt_repair_t unconfined_t:tcp_socket { read setopt write };
 allow passt_repair_t passt_t:tcp_socket { read setopt write };
@@ -80,8 +80,8 @@ allow passt_repair_t passt_t:tcp_socket { read setopt write };
 allow passt_repair_t qemu_var_run_t:unix_stream_socket { connectto read write };
 allow passt_repair_t virt_var_run_t:unix_stream_socket { connectto read write };
 
-allow passt_repair_t qemu_var_run_t:dir search;
-allow passt_repair_t virt_var_run_t:dir search;
+allow passt_repair_t qemu_var_run_t:dir { getattr read search watch };
+allow passt_repair_t virt_var_run_t:dir { getattr read search watch };
 
-allow passt_repair_t qemu_var_run_t:sock_file { read write };
-allow passt_repair_t virt_var_run_t:sock_file { read write };
+allow passt_repair_t qemu_var_run_t:sock_file { getattr read write };
+allow passt_repair_t virt_var_run_t:sock_file { getattr read write };
diff --git a/passt-repair.1 b/passt-repair.1
index 7c1b140..e65aadd 100644
--- a/passt-repair.1
+++ b/passt-repair.1
@@ -16,13 +16,17 @@
 .B passt-repair
 is a privileged helper setting and clearing repair mode on TCP sockets on behalf
 of \fBpasst\fR(1), as instructed via single-byte commands over a UNIX domain
-socket, specified by \fIPATH\fR.
+socket.
 
 It can be used to migrate TCP connections between guests without granting
 additional capabilities to \fBpasst\fR(1) itself: to migrate TCP connections,
 \fBpasst\fR(1) leverages repair mode, which needs the \fBCAP_NET_ADMIN\fR
 capability (see \fBcapabilities\fR(7)) to be set or cleared.
 
+If \fIPATH\fR represents a UNIX domain socket, \fBpasst-repair\fR(1) attempts to
+connect to it. If it is a directory, \fBpasst-repair\fR(1) waits until a file
+ending with \fI.repair\fR appears in it, and then attempts to connect to it.
+
 .SH PROTOCOL
 
 \fBpasst-repair\fR(1) connects to \fBpasst\fR(1) using the socket specified via
diff --git a/passt-repair.c b/passt-repair.c
index e0c366e..8bb3f00 100644
--- a/passt-repair.c
+++ b/passt-repair.c
@@ -16,11 +16,14 @@
  * off. Reply by echoing the command. Exit on EOF.
  */
 
+#include <sys/inotify.h>
 #include <sys/prctl.h>
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/un.h>
 #include <errno.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -39,6 +42,8 @@
 #include "seccomp_repair.h"
 
 #define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */
+#define REPAIR_EXT		".repair"
+#define REPAIR_EXT_LEN		strlen(REPAIR_EXT)
 
 /**
  * main() - Entry point and whole program with loop
@@ -51,6 +56,9 @@
  * #syscalls:repair socket s390x:socketcall i686:socketcall
  * #syscalls:repair recvfrom recvmsg arm:recv ppc64le:recv
  * #syscalls:repair sendto sendmsg arm:send ppc64le:send
+ * #syscalls:repair stat|statx stat64|statx statx
+ * #syscalls:repair fstat|fstat64 newfstatat|fstatat64
+ * #syscalls:repair inotify_init1 inotify_add_watch
  */
 int main(int argc, char **argv)
 {
@@ -58,12 +66,14 @@ int main(int argc, char **argv)
 	     __attribute__ ((aligned(__alignof__(struct cmsghdr))));
 	struct sockaddr_un a = { AF_UNIX, "" };
 	int fds[SCM_MAX_FD], s, ret, i, n = 0;
+	bool inotify_dir = false;
 	struct sock_fprog prog;
 	int8_t cmd = INT8_MAX;
 	struct cmsghdr *cmsg;
 	struct msghdr msg;
 	struct iovec iov;
 	size_t cmsg_len;
+	struct stat sb;
 	int op;
 
 	prctl(PR_SET_DUMPABLE, 0);
@@ -90,19 +100,77 @@ int main(int argc, char **argv)
 		_exit(2);
 	}
 
-	ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
+	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
+		fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno);
+		_exit(1);
+	}
+
+	if ((stat(argv[1], &sb))) {
+		fprintf(stderr, "Can't stat() %s: %i\n", argv[1], errno);
+		_exit(1);
+	}
+
+	if ((sb.st_mode & S_IFMT) == S_IFDIR) {
+		char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
+		const struct inotify_event *ev;
+		char path[PATH_MAX + 1];
+		ssize_t n;
+		int fd;
+
+		ev = (struct inotify_event *)buf;
+
+		if ((fd = inotify_init1(IN_CLOEXEC)) < 0) {
+			fprintf(stderr, "inotify_init1: %i\n", errno);
+			_exit(1);
+		}
+
+		if (inotify_add_watch(fd, argv[1], IN_CREATE) < 0) {
+			fprintf(stderr, "inotify_add_watch: %i\n", errno);
+			_exit(1);
+		}
+
+		do {
+			n = read(fd, buf, sizeof(buf));
+			if (n < 0) {
+				fprintf(stderr, "inotify read: %i", errno);
+				_exit(1);
+			}
+
+			if (n < (ssize_t)sizeof(*ev)) {
+				fprintf(stderr, "Short inotify read: %zi", n);
+				_exit(1);
+			}
+		} while (ev->len < REPAIR_EXT_LEN ||
+			 memcmp(ev->name + strlen(ev->name) - REPAIR_EXT_LEN,
+				REPAIR_EXT, REPAIR_EXT_LEN));
+
+		snprintf(path, sizeof(path), "%s/%s", argv[1], ev->name);
+		if ((stat(path, &sb))) {
+			fprintf(stderr, "Can't stat() %s: %i\n", path, errno);
+			_exit(1);
+		}
+
+		ret = snprintf(a.sun_path, sizeof(a.sun_path), path);
+		inotify_dir = true;
+	} else {
+		ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
+	}
+
 	if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) {
-		fprintf(stderr, "Invalid socket path: %s\n", argv[1]);
+		fprintf(stderr, "Invalid socket path");
 		_exit(2);
 	}
 
-	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
-		fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno);
-		_exit(1);
+	if ((sb.st_mode & S_IFMT) != S_IFSOCK) {
+		fprintf(stderr, "%s is not a socket\n", a.sun_path);
+		_exit(2);
 	}
 
-	if (connect(s, (struct sockaddr *)&a, sizeof(a))) {
-		fprintf(stderr, "Failed to connect to %s: %s\n", argv[1],
+	while (connect(s, (struct sockaddr *)&a, sizeof(a))) {
+		if (inotify_dir && errno == ECONNREFUSED)
+			continue;
+
+		fprintf(stderr, "Failed to connect to %s: %s\n", a.sun_path,
 			strerror(errno));
 		_exit(1);
 	}
-- 
@@ -16,11 +16,14 @@
  * off. Reply by echoing the command. Exit on EOF.
  */
 
+#include <sys/inotify.h>
 #include <sys/prctl.h>
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/un.h>
 #include <errno.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -39,6 +42,8 @@
 #include "seccomp_repair.h"
 
 #define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */
+#define REPAIR_EXT		".repair"
+#define REPAIR_EXT_LEN		strlen(REPAIR_EXT)
 
 /**
  * main() - Entry point and whole program with loop
@@ -51,6 +56,9 @@
  * #syscalls:repair socket s390x:socketcall i686:socketcall
  * #syscalls:repair recvfrom recvmsg arm:recv ppc64le:recv
  * #syscalls:repair sendto sendmsg arm:send ppc64le:send
+ * #syscalls:repair stat|statx stat64|statx statx
+ * #syscalls:repair fstat|fstat64 newfstatat|fstatat64
+ * #syscalls:repair inotify_init1 inotify_add_watch
  */
 int main(int argc, char **argv)
 {
@@ -58,12 +66,14 @@ int main(int argc, char **argv)
 	     __attribute__ ((aligned(__alignof__(struct cmsghdr))));
 	struct sockaddr_un a = { AF_UNIX, "" };
 	int fds[SCM_MAX_FD], s, ret, i, n = 0;
+	bool inotify_dir = false;
 	struct sock_fprog prog;
 	int8_t cmd = INT8_MAX;
 	struct cmsghdr *cmsg;
 	struct msghdr msg;
 	struct iovec iov;
 	size_t cmsg_len;
+	struct stat sb;
 	int op;
 
 	prctl(PR_SET_DUMPABLE, 0);
@@ -90,19 +100,77 @@ int main(int argc, char **argv)
 		_exit(2);
 	}
 
-	ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
+	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
+		fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno);
+		_exit(1);
+	}
+
+	if ((stat(argv[1], &sb))) {
+		fprintf(stderr, "Can't stat() %s: %i\n", argv[1], errno);
+		_exit(1);
+	}
+
+	if ((sb.st_mode & S_IFMT) == S_IFDIR) {
+		char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
+		const struct inotify_event *ev;
+		char path[PATH_MAX + 1];
+		ssize_t n;
+		int fd;
+
+		ev = (struct inotify_event *)buf;
+
+		if ((fd = inotify_init1(IN_CLOEXEC)) < 0) {
+			fprintf(stderr, "inotify_init1: %i\n", errno);
+			_exit(1);
+		}
+
+		if (inotify_add_watch(fd, argv[1], IN_CREATE) < 0) {
+			fprintf(stderr, "inotify_add_watch: %i\n", errno);
+			_exit(1);
+		}
+
+		do {
+			n = read(fd, buf, sizeof(buf));
+			if (n < 0) {
+				fprintf(stderr, "inotify read: %i", errno);
+				_exit(1);
+			}
+
+			if (n < (ssize_t)sizeof(*ev)) {
+				fprintf(stderr, "Short inotify read: %zi", n);
+				_exit(1);
+			}
+		} while (ev->len < REPAIR_EXT_LEN ||
+			 memcmp(ev->name + strlen(ev->name) - REPAIR_EXT_LEN,
+				REPAIR_EXT, REPAIR_EXT_LEN));
+
+		snprintf(path, sizeof(path), "%s/%s", argv[1], ev->name);
+		if ((stat(path, &sb))) {
+			fprintf(stderr, "Can't stat() %s: %i\n", path, errno);
+			_exit(1);
+		}
+
+		ret = snprintf(a.sun_path, sizeof(a.sun_path), path);
+		inotify_dir = true;
+	} else {
+		ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
+	}
+
 	if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) {
-		fprintf(stderr, "Invalid socket path: %s\n", argv[1]);
+		fprintf(stderr, "Invalid socket path");
 		_exit(2);
 	}
 
-	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
-		fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno);
-		_exit(1);
+	if ((sb.st_mode & S_IFMT) != S_IFSOCK) {
+		fprintf(stderr, "%s is not a socket\n", a.sun_path);
+		_exit(2);
 	}
 
-	if (connect(s, (struct sockaddr *)&a, sizeof(a))) {
-		fprintf(stderr, "Failed to connect to %s: %s\n", argv[1],
+	while (connect(s, (struct sockaddr *)&a, sizeof(a))) {
+		if (inotify_dir && errno == ECONNREFUSED)
+			continue;
+
+		fprintf(stderr, "Failed to connect to %s: %s\n", a.sun_path,
 			strerror(errno));
 		_exit(1);
 	}
-- 
2.43.0

Re: [PATCH] passt-repair: Add directory watch

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 8909 bytes --]

On Fri, Mar 07, 2025 at 11:41:20PM +0100, Stefano Brivio wrote:
> It might not be feasible for users to start passt-repair after passt
> is started, on a migration target, but before the migration process
> starts.
> 
> For instance, with libvirt, the guest domain (and, hence, passt) is
> started on the target as part of the migration process. At least for
> the moment being, there's no hook a libvirt user (including KubeVirt)
> can use to start passt-repair before the migration starts.
> 
> Add a directory watch using inotify: if PATH is a directory, instead
> of connecting to it, we'll watch for a .repair socket file to appear
> in it, and then attempt to connect to that socket.

So, with this change, running
	passt-repair /tmp

would be a Bad Idea.  But that is the default path used by passt.  To
use this safely, you really want to have a directory set aside for the
use of just one passt instance, or at least passt-owning uid.

I feel like we should enforce, or at least document and encourage that
somewhere.  Not really sure where, though, so, with some misgivings

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  contrib/selinux/passt-repair.te | 16 +++----
>  passt-repair.1                  |  6 ++-
>  passt-repair.c                  | 82 ++++++++++++++++++++++++++++++---
>  3 files changed, 88 insertions(+), 16 deletions(-)
> 
> diff --git a/contrib/selinux/passt-repair.te b/contrib/selinux/passt-repair.te
> index f171be6..7157dfb 100644
> --- a/contrib/selinux/passt-repair.te
> +++ b/contrib/selinux/passt-repair.te
> @@ -61,11 +61,11 @@ allow passt_repair_t unconfined_t:unix_stream_socket { connectto read write };
>  allow passt_repair_t passt_t:unix_stream_socket { connectto read write };
>  allow passt_repair_t user_tmp_t:unix_stream_socket { connectto read write };
>  
> -allow passt_repair_t user_tmp_t:dir search;
> +allow passt_repair_t user_tmp_t:dir { getattr read search watch };
>  
> -allow passt_repair_t unconfined_t:sock_file { read write };
> -allow passt_repair_t passt_t:sock_file { read write };
> -allow passt_repair_t user_tmp_t:sock_file { read write };
> +allow passt_repair_t unconfined_t:sock_file { getattr read write };
> +allow passt_repair_t passt_t:sock_file { getattr read write };
> +allow passt_repair_t user_tmp_t:sock_file { getattr read write };
>  
>  allow passt_repair_t unconfined_t:tcp_socket { read setopt write };
>  allow passt_repair_t passt_t:tcp_socket { read setopt write };
> @@ -80,8 +80,8 @@ allow passt_repair_t passt_t:tcp_socket { read setopt write };
>  allow passt_repair_t qemu_var_run_t:unix_stream_socket { connectto read write };
>  allow passt_repair_t virt_var_run_t:unix_stream_socket { connectto read write };
>  
> -allow passt_repair_t qemu_var_run_t:dir search;
> -allow passt_repair_t virt_var_run_t:dir search;
> +allow passt_repair_t qemu_var_run_t:dir { getattr read search watch };
> +allow passt_repair_t virt_var_run_t:dir { getattr read search watch };
>  
> -allow passt_repair_t qemu_var_run_t:sock_file { read write };
> -allow passt_repair_t virt_var_run_t:sock_file { read write };
> +allow passt_repair_t qemu_var_run_t:sock_file { getattr read write };
> +allow passt_repair_t virt_var_run_t:sock_file { getattr read write };
> diff --git a/passt-repair.1 b/passt-repair.1
> index 7c1b140..e65aadd 100644
> --- a/passt-repair.1
> +++ b/passt-repair.1
> @@ -16,13 +16,17 @@
>  .B passt-repair
>  is a privileged helper setting and clearing repair mode on TCP sockets on behalf
>  of \fBpasst\fR(1), as instructed via single-byte commands over a UNIX domain
> -socket, specified by \fIPATH\fR.
> +socket.
>  
>  It can be used to migrate TCP connections between guests without granting
>  additional capabilities to \fBpasst\fR(1) itself: to migrate TCP connections,
>  \fBpasst\fR(1) leverages repair mode, which needs the \fBCAP_NET_ADMIN\fR
>  capability (see \fBcapabilities\fR(7)) to be set or cleared.
>  
> +If \fIPATH\fR represents a UNIX domain socket, \fBpasst-repair\fR(1) attempts to
> +connect to it. If it is a directory, \fBpasst-repair\fR(1) waits until a file
> +ending with \fI.repair\fR appears in it, and then attempts to connect to it.
> +
>  .SH PROTOCOL
>  
>  \fBpasst-repair\fR(1) connects to \fBpasst\fR(1) using the socket specified via
> diff --git a/passt-repair.c b/passt-repair.c
> index e0c366e..8bb3f00 100644
> --- a/passt-repair.c
> +++ b/passt-repair.c
> @@ -16,11 +16,14 @@
>   * off. Reply by echoing the command. Exit on EOF.
>   */
>  
> +#include <sys/inotify.h>
>  #include <sys/prctl.h>
>  #include <sys/types.h>
>  #include <sys/socket.h>
> +#include <sys/stat.h>
>  #include <sys/un.h>
>  #include <errno.h>
> +#include <stdbool.h>
>  #include <stddef.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -39,6 +42,8 @@
>  #include "seccomp_repair.h"
>  
>  #define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */
> +#define REPAIR_EXT		".repair"
> +#define REPAIR_EXT_LEN		strlen(REPAIR_EXT)
>  
>  /**
>   * main() - Entry point and whole program with loop
> @@ -51,6 +56,9 @@
>   * #syscalls:repair socket s390x:socketcall i686:socketcall
>   * #syscalls:repair recvfrom recvmsg arm:recv ppc64le:recv
>   * #syscalls:repair sendto sendmsg arm:send ppc64le:send
> + * #syscalls:repair stat|statx stat64|statx statx
> + * #syscalls:repair fstat|fstat64 newfstatat|fstatat64
> + * #syscalls:repair inotify_init1 inotify_add_watch
>   */
>  int main(int argc, char **argv)
>  {
> @@ -58,12 +66,14 @@ int main(int argc, char **argv)
>  	     __attribute__ ((aligned(__alignof__(struct cmsghdr))));
>  	struct sockaddr_un a = { AF_UNIX, "" };
>  	int fds[SCM_MAX_FD], s, ret, i, n = 0;
> +	bool inotify_dir = false;
>  	struct sock_fprog prog;
>  	int8_t cmd = INT8_MAX;
>  	struct cmsghdr *cmsg;
>  	struct msghdr msg;
>  	struct iovec iov;
>  	size_t cmsg_len;
> +	struct stat sb;
>  	int op;
>  
>  	prctl(PR_SET_DUMPABLE, 0);
> @@ -90,19 +100,77 @@ int main(int argc, char **argv)
>  		_exit(2);
>  	}
>  
> -	ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
> +	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
> +		fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno);
> +		_exit(1);
> +	}
> +
> +	if ((stat(argv[1], &sb))) {
> +		fprintf(stderr, "Can't stat() %s: %i\n", argv[1], errno);
> +		_exit(1);
> +	}
> +
> +	if ((sb.st_mode & S_IFMT) == S_IFDIR) {
> +		char buf[sizeof(struct inotify_event) + NAME_MAX + 1];
> +		const struct inotify_event *ev;
> +		char path[PATH_MAX + 1];
> +		ssize_t n;
> +		int fd;
> +
> +		ev = (struct inotify_event *)buf;
> +
> +		if ((fd = inotify_init1(IN_CLOEXEC)) < 0) {
> +			fprintf(stderr, "inotify_init1: %i\n", errno);
> +			_exit(1);
> +		}
> +
> +		if (inotify_add_watch(fd, argv[1], IN_CREATE) < 0) {
> +			fprintf(stderr, "inotify_add_watch: %i\n", errno);
> +			_exit(1);
> +		}
> +
> +		do {
> +			n = read(fd, buf, sizeof(buf));
> +			if (n < 0) {
> +				fprintf(stderr, "inotify read: %i", errno);
> +				_exit(1);
> +			}
> +
> +			if (n < (ssize_t)sizeof(*ev)) {
> +				fprintf(stderr, "Short inotify read: %zi", n);
> +				_exit(1);
> +			}
> +		} while (ev->len < REPAIR_EXT_LEN ||
> +			 memcmp(ev->name + strlen(ev->name) - REPAIR_EXT_LEN,
> +				REPAIR_EXT, REPAIR_EXT_LEN));
> +
> +		snprintf(path, sizeof(path), "%s/%s", argv[1], ev->name);
> +		if ((stat(path, &sb))) {
> +			fprintf(stderr, "Can't stat() %s: %i\n", path, errno);
> +			_exit(1);
> +		}
> +
> +		ret = snprintf(a.sun_path, sizeof(a.sun_path), path);
> +		inotify_dir = true;
> +	} else {
> +		ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
> +	}
> +
>  	if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) {
> -		fprintf(stderr, "Invalid socket path: %s\n", argv[1]);
> +		fprintf(stderr, "Invalid socket path");
>  		_exit(2);
>  	}
>  
> -	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
> -		fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno);
> -		_exit(1);
> +	if ((sb.st_mode & S_IFMT) != S_IFSOCK) {
> +		fprintf(stderr, "%s is not a socket\n", a.sun_path);
> +		_exit(2);
>  	}
>  
> -	if (connect(s, (struct sockaddr *)&a, sizeof(a))) {
> -		fprintf(stderr, "Failed to connect to %s: %s\n", argv[1],
> +	while (connect(s, (struct sockaddr *)&a, sizeof(a))) {
> +		if (inotify_dir && errno == ECONNREFUSED)
> +			continue;
> +
> +		fprintf(stderr, "Failed to connect to %s: %s\n", a.sun_path,
>  			strerror(errno));
>  		_exit(1);
>  	}

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] passt-repair: Add directory watch

[Stefano Brivio]

On Tue, 11 Mar 2025 12:35:46 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Mar 07, 2025 at 11:41:20PM +0100, Stefano Brivio wrote:
> > It might not be feasible for users to start passt-repair after passt
> > is started, on a migration target, but before the migration process
> > starts.
> > 
> > For instance, with libvirt, the guest domain (and, hence, passt) is
> > started on the target as part of the migration process. At least for
> > the moment being, there's no hook a libvirt user (including KubeVirt)
> > can use to start passt-repair before the migration starts.
> > 
> > Add a directory watch using inotify: if PATH is a directory, instead
> > of connecting to it, we'll watch for a .repair socket file to appear
> > in it, and then attempt to connect to that socket.  
> 
> So, with this change, running
> 	passt-repair /tmp
> 
> would be a Bad Idea.

...why? On any distribution where it's available, you can make it
connect to whatever you want, and it will do nothing else than
returning an error when passt tries to switch a socket to repair mode.

It will just work in the KubeVirt use case we planned for, for the
moment.

Then sure, you can give it capabilities or run it as root, disable
LSMs, and make it connect to whatever process. But you need root
anyway, so there isn't much to be gained.

> But that is the default path used by passt.  To
> use this safely, you really want to have a directory set aside for the
> use of just one passt instance, or at least passt-owning uid.

Right, that's what happens if libvirt starts it.

> I feel like we should enforce, or at least document and encourage that
> somewhere.  Not really sure where, though, so, with some misgivings

I think we'll find a more reasonable solution by the time this becomes
actually usable by mere mortals using distribution packages. I would
anyway drop all this once we figure out how to make it convenient for
libvirt. For stand-alone usage, this is not really needed.

-- 
Stefano

Re: [PATCH] passt-repair: Add directory watch

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2614 bytes --]

On Tue, Mar 11, 2025 at 11:44:57PM +0100, Stefano Brivio wrote:
> On Tue, 11 Mar 2025 12:35:46 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, Mar 07, 2025 at 11:41:20PM +0100, Stefano Brivio wrote:
> > > It might not be feasible for users to start passt-repair after passt
> > > is started, on a migration target, but before the migration process
> > > starts.
> > > 
> > > For instance, with libvirt, the guest domain (and, hence, passt) is
> > > started on the target as part of the migration process. At least for
> > > the moment being, there's no hook a libvirt user (including KubeVirt)
> > > can use to start passt-repair before the migration starts.
> > > 
> > > Add a directory watch using inotify: if PATH is a directory, instead
> > > of connecting to it, we'll watch for a .repair socket file to appear
> > > in it, and then attempt to connect to that socket.  
> > 
> > So, with this change, running
> > 	passt-repair /tmp
> > 
> > would be a Bad Idea.
> 
> ...why? On any distribution where it's available, you can make it
> connect to whatever you want, and it will do nothing else than
> returning an error when passt tries to switch a socket to repair
> mode.

I mean that if you are able to run it privileged, then it would be a
bad idea to do so with /tmp as the watch directory.  Since that's kind
of the default place to point it, it's a footgun.

> It will just work in the KubeVirt use case we planned for, for the
> moment.
> 
> Then sure, you can give it capabilities or run it as root, disable
> LSMs, and make it connect to whatever process. But you need root
> anyway, so there isn't much to be gained.
> 
> > But that is the default path used by passt.  To
> > use this safely, you really want to have a directory set aside for the
> > use of just one passt instance, or at least passt-owning uid.
> 
> Right, that's what happens if libvirt starts it.
> 
> > I feel like we should enforce, or at least document and encourage that
> > somewhere.  Not really sure where, though, so, with some misgivings
> 
> I think we'll find a more reasonable solution by the time this becomes
> actually usable by mere mortals using distribution packages. I would
> anyway drop all this once we figure out how to make it convenient for
> libvirt. For stand-alone usage, this is not really needed.

Hm.  I guess we'll see.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]