From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202502 header.b=RrFVHFQM; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 7C0B45A061A for ; Tue, 11 Mar 2025 02:36:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1741656964; bh=uz3xgfEb0w4DQZaSMa836AoijfYoOUYHyBGa/fWS2Nc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RrFVHFQMc6jD6ZNEekaBYH+X/GSBkC+bjewU7ES3mBFwswnZ5zGBi+QwWmqc1QI7e hGPC+edPtQXgbhRfBPR/8n7iahqAiYQM/pGEiF1UwtQeptGfW6k0ThNpmaq3jGai17 mJxjEQvzVLxoS/kTPC6ffac2/P3zKmW0e6c/VcZu3GToIMFfHakowmbAt2s11p7rnh 2dcREKJajBES5Hk7gEZbLmA4ZcG6LoLbai0p4Fh/pZMRjvRSUqTmolY4Sf3beewzSl Yuubytek8y1PYigx5uiXOfsaOC8rWCOhhLkotww9iGfzEj5uaSfsIk48OrfDGZZoqv f3vUOH+BK/oiA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZBbv866VGz4xCW; Tue, 11 Mar 2025 12:36:04 +1100 (AEDT) Date: Tue, 11 Mar 2025 12:35:46 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] passt-repair: Add directory watch Message-ID: References: <20250307224120.2789900-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Iqk9HtkWJ5lGrMpC" Content-Disposition: inline In-Reply-To: <20250307224120.2789900-1-sbrivio@redhat.com> Message-ID-Hash: 5PXRCZIPL5IBDT3CR5QEGQG2VWVVDDSC X-Message-ID-Hash: 5PXRCZIPL5IBDT3CR5QEGQG2VWVVDDSC X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --Iqk9HtkWJ5lGrMpC Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > 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. >=20 > 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(-) >=20 > diff --git a/contrib/selinux/passt-repair.te b/contrib/selinux/passt-repa= ir.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 writ= e }; > =20 > -allow passt_repair_t user_tmp_t:dir search; > +allow passt_repair_t user_tmp_t:dir { getattr read search watch }; > =20 > -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 }; > =20 > 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 w= rite }; > 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 }; > =20 > -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 }; > =20 > -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 o= n behalf > of \fBpasst\fR(1), as instructed via single-byte commands over a UNIX do= main > -socket, specified by \fIPATH\fR. > +socket. > =20 > It can be used to migrate TCP connections between guests without granting > additional capabilities to \fBpasst\fR(1) itself: to migrate TCP connect= ions, > \fBpasst\fR(1) leverages repair mode, which needs the \fBCAP_NET_ADMIN\fR > capability (see \fBcapabilities\fR(7)) to be set or cleared. > =20 > +If \fIPATH\fR represents a UNIX domain socket, \fBpasst-repair\fR(1) att= empts 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 > =20 > \fBpasst-repair\fR(1) connects to \fBpasst\fR(1) using the socket specif= ied 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. > */ > =20 > +#include > #include > #include > #include > +#include > #include > #include > +#include > #include > #include > #include > @@ -39,6 +42,8 @@ > #include "seccomp_repair.h" > =20 > #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) > =20 > /** > * 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 =3D { AF_UNIX, "" }; > int fds[SCM_MAX_FD], s, ret, i, n =3D 0; > + bool inotify_dir =3D false; > struct sock_fprog prog; > int8_t cmd =3D INT8_MAX; > struct cmsghdr *cmsg; > struct msghdr msg; > struct iovec iov; > size_t cmsg_len; > + struct stat sb; > int op; > =20 > prctl(PR_SET_DUMPABLE, 0); > @@ -90,19 +100,77 @@ int main(int argc, char **argv) > _exit(2); > } > =20 > - ret =3D snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]); > + if ((s =3D 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) =3D=3D 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 =3D (struct inotify_event *)buf; > + > + if ((fd =3D 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 =3D 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 =3D snprintf(a.sun_path, sizeof(a.sun_path), path); > + inotify_dir =3D true; > + } else { > + ret =3D snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]); > + } > + > if (ret <=3D 0 || ret >=3D (int)sizeof(a.sun_path)) { > - fprintf(stderr, "Invalid socket path: %s\n", argv[1]); > + fprintf(stderr, "Invalid socket path"); > _exit(2); > } > =20 > - if ((s =3D 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) !=3D S_IFSOCK) { > + fprintf(stderr, "%s is not a socket\n", a.sun_path); > + _exit(2); > } > =20 > - 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 =3D=3D ECONNREFUSED) > + continue; > + > + fprintf(stderr, "Failed to connect to %s: %s\n", a.sun_path, > strerror(errno)); > _exit(1); > } --=20 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 --Iqk9HtkWJ5lGrMpC Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmfPk3EACgkQzQJF27ox 2Geh7A//alPgzyVMJBC4tZ8wGrLWpnA4uSrvv1jhJQvHbuTj8CBPafCAD1CaFLr8 RCzzUI+ol6fXSo4KxPqmMLsLWr61AFraMMPuBdtl5o5G5RC7pSdxN7Ii0RxDG/Uv BTN2FmXIK4jGMMEDHl1zX5vB8sP8ebt9j0GGBQmdGVyyPkSCo1p7p4XV8XMBc/PO oyMWBed/oCr7v/pM8Yi9iXZLTGCd7eDozPa6I+Gx9aQmn6kB1JHU7BaOIHi/wNvz tGcPTZ2M8O9X6bUib7ExR78/g4ywxj9dOLDyRTpW3TqJkd8UJr7AXt9HMRJTjwWa 5R8SrUsV20HQrbsCowkz3MCj8x7FM4UM/TVuaOYPdlbgvsx6S824TzHW+EH/Wre4 6Pyap4FEKdXzR2Od8B2z7TYZeIZwiPRxoZxP3h8k8694UgOh3AglU/gvOk07sh/T qVCKexhxxCYlBRswgrk03SBYxB1L/ozZJfhcKb/cScRSWsh5zLnusdcZm6Tlwliq 0+kDgJtqYdH0gl5rbLW0LWxAY2klWTnU297PQ93ioUFy5+XzcmmKGeNTHP6NfH5T uAOKYHcegBxf/hfcWzEfU2VXHNZcWNN82/4X6aCxiNSCvkvUgoXWOC4RT9Zl7iJw JceW24XNqHzVaHodEULd9gLOgKBe/kE3e+IdY+7c6g6nzusTmdQ= =otid -----END PGP SIGNATURE----- --Iqk9HtkWJ5lGrMpC--