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 > Signed-off-by: Stefano Brivio > --- > 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 > #include > #include > #include > +#include > #include > #include > +#include > #include > #include > #include > @@ -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