- dev - passt

[PATCH] selinux: Fixes/workarounds for passt and passt-repair, mostly for libvirt usage
by Stefano Brivio 28 Feb '25

28 Feb '25

Here are a bunch of workarounds and a couple of fixes for libvirt usage which are rather hard to split into single logical patches as there appear to be some obscure dependencies between some of them: - passt-repair needs to have an exec_type typeattribute (otherwise the policy for lsmd(1) causes a violation on getattr on its executable) file, and that typeattribute just happened to be there for passt as a result of init_daemon_domain(), but passt-repair isn't a daemon, so we need an explicit corecmd_executable_file() - passt-repair needs a workaround, which I'll revisit once https://github.com/fedora-selinux/selinux-policy/issues/2579 is solved, for usage with libvirt: allow it to use qemu_var_run_t and virt_var_run_t sockets - add 'bpf' and 'dac_read_search' capabilities for passt-repair: they are needed (for whatever reason I didn't investigate) to actually receive socket files via SCM_RIGHTS - passt needs further workarounds in the sense of https://github.com/fedora-selinux/selinux-policy/issues/2579: allow it to use map and use svirt_tmpfs_t (not just svirt_image_t): it depends on where the libvirt guest image is - ...it also needs to map /dev/null if <access mode='shared'/> is enabled in libvirt's XML for the memoryBacking object, for vhost-user operation - and 'ioctl' on the TCP socket appears to be actually needed, on top of 'getattr', to dump some socket parameters Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- contrib/selinux/passt-repair.te | 33 +++++++++++++++++++++++++++++++-- contrib/selinux/passt.te | 9 +++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/contrib/selinux/passt-repair.te b/contrib/selinux/passt-repair.te index e3ffbcd..f171be6 100644 --- a/contrib/selinux/passt-repair.te +++ b/contrib/selinux/passt-repair.te @@ -28,12 +28,22 @@ require { type console_device_t; type user_devpts_t; type user_tmp_t; + + # Workaround: passt-repair needs to needs to access socket files + # that passt, started by libvirt, might create under different + # labels, depending on whether passt is started as root or not. + # + # However, libvirt doesn't maintain its own policy, which makes + # updates particularly complicated. To avoid breakage in the short + # term, deal with that in passt's own policy. + type qemu_var_run_t; + type virt_var_run_t; } type passt_repair_t; domain_type(passt_repair_t); type passt_repair_exec_t; -files_type(passt_repair_exec_t); +corecmd_executable_file(passt_repair_exec_t); role unconfined_r types passt_repair_t; @@ -41,7 +51,8 @@ allow passt_repair_t passt_repair_exec_t:file { read execute execute_no_trans en type_transition unconfined_t passt_repair_exec_t:process passt_repair_t; allow unconfined_t passt_repair_t:process transition; -allow passt_repair_t self:capability { dac_override net_admin net_raw }; +allow passt_repair_t self:capability { dac_override dac_read_search net_admin net_raw }; +allow passt_repair_t self:capability2 bpf; allow passt_repair_t console_device_t:chr_file { append open getattr read write ioctl }; allow passt_repair_t user_devpts_t:chr_file { append open getattr read write ioctl }; @@ -50,9 +61,27 @@ 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 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:tcp_socket { read setopt write }; allow passt_repair_t passt_t:tcp_socket { read setopt write }; + +# Workaround: passt-repair needs to needs to access socket files +# that passt, started by libvirt, might create under different +# labels, depending on whether passt is started as root or not. +# +# However, libvirt doesn't maintain its own policy, which makes +# updates particularly complicated. To avoid breakage in the short +# term, deal with that in passt's own policy. +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:sock_file { read write }; +allow passt_repair_t virt_var_run_t:sock_file { read write }; diff --git a/contrib/selinux/passt.te b/contrib/selinux/passt.te index f595079..f8ea672 100644 --- a/contrib/selinux/passt.te +++ b/contrib/selinux/passt.te @@ -29,6 +29,9 @@ require { # particularly complicated. To avoid breakage in the short term, # deal with it in passt's own policy. type svirt_image_t; + type svirt_tmpfs_t; + type svirt_t; + type null_device_t; class file { ioctl getattr setattr create read write unlink open relabelto execute execute_no_trans map }; class dir { search write add_name remove_name mounton }; @@ -45,7 +48,7 @@ require { type net_conf_t; type proc_net_t; type node_t; - class tcp_socket { create accept listen name_bind name_connect getattr }; + class tcp_socket { create accept listen name_bind name_connect getattr ioctl }; class udp_socket { create accept listen }; class icmp_socket { bind create name_bind node_bind setopt read write }; class sock_file { create unlink write }; @@ -129,7 +132,7 @@ corenet_udp_sendrecv_all_ports(passt_t) allow passt_t node_t:icmp_socket { name_bind node_bind }; allow passt_t port_t:icmp_socket name_bind; -allow passt_t self:tcp_socket { create getopt setopt connect bind listen accept shutdown read write getattr }; +allow passt_t self:tcp_socket { create getopt setopt connect bind listen accept shutdown read write getattr ioctl }; allow passt_t self:udp_socket { create getopt setopt connect bind read write }; allow passt_t self:icmp_socket { bind create setopt read write }; @@ -143,3 +146,5 @@ allow passt_t unconfined_t:unix_stream_socket { read write }; # particularly complicated. To avoid breakage in the short term, # deal with it in passt's own policy. allow passt_t svirt_image_t:file { read write map }; +allow passt_t svirt_tmpfs_t:file { read write map }; +allow passt_t null_device_t:chr_file map; -- 2.43.0

1 0

tcp_rst() complications
by David Gibson 27 Feb '25

27 Feb '25

Amongst other things, I spotted some additional complications using tcp_rst() in the migration path (some of which might also have implications in other contexts). These might be things we can safely ignore, at least for now, but I haven't thought through them enough to be sure. 1) Sending RST to guest during migration The first issue is that tcp_rst() will send an actual RST to the guest on the tap interface. During migration, that means we're sending to the guest while it's suspended. At the very least that means we probably have a much higher that usual chance of getting a queue full failure writing to the tap interface, which could hit problem (2). But, beyond that, with vhost-user that means we're writing to guest memory while the guest is suspended. Kind of the whole point of the suspended time is that the guest memory doesn't change during it, so I'm not sure what the consequences will be. Now, at the moment I think all our tcp_rst() calls are either on the source during rollback (i.e. we're committed to resuming only on the source) or on the target past the point of no return (i.e. we're committed to resuming only on the target). I suspect that means we can get away with it, but I do worry this could break something in qeme by violating that assumption. 2) tcp_rst() failures tcp_rst() can fail if tcp_send_flag() fails. In this case we *don't* change the events to CLOSED. I _think_ that's a bug: even if we weren't able to send the RST to the guest, we've already closed the socket so the flow is dead. Moving to CLOSED state (and then removing the flow entirely) should mean that we'll resend an RST if the guest attempts to use the flow again later. But.. I was worried there might be some subtle reason for not changing the event state in that case. -- 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

2 5

[PATCH v2 0/2] More graceful handling of migration without passt-repair
by David Gibson 27 Feb '25

27 Feb '25

From Red Hat internal testing we've had some reports that if attempting to migrate without passt-repair, the failure mode is uglier than we'd like. The migration fails, which is somewhat expected, but we don't correctly roll things back on the source, so it breaks network there as well. Handle this more gracefully allowing the migration to proceed in this case, but allow TCP connections to break I've now tested this reasonably: * I get a clean migration if there are now active flows * Migration completes, although connections are broken if passt-repair isn't connected * Basic test suite (minus perf) I didn't manage to test with libvirt yet, but I'm pretty convinced the behaviour should be better than it was. There are more fragile cases that I'm looking to fix, particularly the die()s in flow_migrate_source_rollback() and elsewhere, however I ran into various complications that I didn't manage to sort out today. I'll continue looking at those tomorrow. I'm now pretty confident that those additional fixes won't entirely supersede the changes in this series, so it should be fine to apply these on their own. David Gibson (2): migrate, flow: Trivially succeed if migrating with no flows migrate, flow: Don't attempt to migrate TCP flows without passt-repair flow.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) -- 2.48.1

2 9

[PATCH v3 0/3] More graceful handling of migration without passt-repair
by David Gibson 27 Feb '25

27 Feb '25

From Red Hat internal testing we've had some reports that if attempting to migrate without passt-repair, the failure mode is uglier than we'd like. The migration fails, which is somewhat expected, but we don't correctly roll things back on the source, so it breaks network there as well. Handle this more gracefully allowing the migration to proceed in this case, but allow TCP connections to break I've test patches 1..2/3 reasonably: * I get a clean migration if there are now active flows * Migration completes, although connections are broken if passt-repair isn't connected * Basic test suite (minus perf) Patch 3 I've run the basic test suite on, but haven't tested the specific functionality of. Alas, I've spent most of today battling with RHEL, virt-install, unshare and various other things trying to create a test environment simulating two hosts with (possibly) different addresses. Despite the example given by Stefano in reply to the previous version, I haven't really succeeded yet. There are more fragile cases that I'm looking to fix, particularly the die()s in flow_migrate_source_rollback() and elsewhere, however I ran into various complications that I didn't manage to sort out today. I'll continue looking at those tomorrow. David Gibson (3): migrate, flow: Trivially succeed if migrating with no flows migrate, flow: Don't attempt to migrate TCP flows without passt-repair migrate, tcp: Don't attempt to carry on migration after flow_alloc_cancel() flow.c | 17 +++++++++++++++-- tcp.c | 5 ++++- 2 files changed, 19 insertions(+), 3 deletions(-) -- 2.48.1

2 5

[PATCH v6 0/4] Reconstruct ICMP headers for failed UDP connect
by Jon Maloy 26 Feb '25

26 Feb '25

Reconstruct incoming ICMP headers for failed UDP connect and forward back to local peer. v2: - Added patch breaking out udp header creation from function tap_udp4_send(). - Updated the ICMP creation by using the new function. - Added logics to find correct flow, depending on origin. - All done after feedback from David Gibson. v3: - More changes after feedback from David Gibson. v4: - Even more changes after feedback from D. Gibson v5: - Added corresponding patches for IPv6 v6: - Fixed some small nits after comments from D. Gibson. Jon Maloy (4): tap: break out building of udp header from tap_udp4_send function udp: create and send ICMPv4 to local peer when applicable tap: break out building of udp header from tap_udp6_send function udp: create and send ICMPv6 to local peer when applicable tap.c | 86 +++++++++++++++++++++++++---------- tap.h | 15 ++++++ udp.c | 121 ++++++++++++++++++++++++++++++++++++++++++++----- udp_internal.h | 2 +- udp_vu.c | 4 +- 5 files changed, 189 insertions(+), 39 deletions(-) -- 2.48.1

2 7

[PATCH] seccomp.sh: Silence stty errors
by Michal Privoznik 24 Feb '25

24 Feb '25

When printing list of allowed syscalls the width of terminal is obtained for nicer output (see commit below). The width is obtained by running 'stty'. While this works when building from a console, it doesn't work during rpmbuild/emerge/.. as stdout is usually not a console but a logfile and stdin is usually /dev/null or something. This results in stty reporting errors like this: stty: 'standard input': Inappropriate ioctl for device Redirect stty's stderr to /dev/null to silence it. Fixes: 712ca3235329b049bf9a4e481ba38a4c64768e8b Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- I've noticed the error when running emerge on my local system, but then also found it on a recent koji build: https://kojipkgs.fedoraproject.org//packages/passt/0%5E20250217.ga1e48a0/2.… seccomp.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/seccomp.sh b/seccomp.sh index 4c521ae..a7bc417 100755 --- a/seccomp.sh +++ b/seccomp.sh @@ -255,7 +255,7 @@ for __p in ${__profiles}; do __calls="${__calls} ${EXTRA_SYSCALLS:-}" __calls="$(filter ${__calls})" - cols="$(stty -a | sed -n 's/.*columns $[0-9]*$.*/\1/p' || :)" 2>/dev/null + cols="$(stty -a 2>/dev/null | sed -n 's/.*columns $[0-9]*$.*/\1/p' || :)" 2>/dev/null case $cols in [0-9]*) col_args="-w ${cols}";; *) col_args="";; esac echo "seccomp profile ${__p} allows: ${__calls}" | tr '\n' ' ' | fmt -t ${col_args} -- 2.45.3

3 4

[PATCH 0/3] Tweaks to mode handling
by David Gibson 24 Feb '25

24 Feb '25

While working on something else, I got mildly irritated by the fact that there's no a consistent place after which we can rely on the mode. We detect passt vs. pasta early, but only split out vhost-user later, which forces some other things later still. David Gibson (3): conf: Move mode detection into helper function conf: Detect vhost-user mode earlier conf: Use the same optstring for passt and pasta modes conf.c | 87 ++++++++++++++++++++++++++++++++++++++++++--------------- conf.h | 1 + passt.c | 14 ++-------- 3 files changed, 68 insertions(+), 34 deletions(-) -- 2.48.1

1 3

[PATCH v5 0/4] Reconstruct ICMP headers for failed UDP connect
by Jon Maloy 24 Feb '25

24 Feb '25

Reconstruct incoming ICMP headers for failed UDP connect and forward back to local peer. v2: - Added patch breaking out udp header creation from function tap_udp4_send(). - Updated the ICMP creation by using the new function. - Added logics to find correct flow, depending on origin. - All done after feedback from David Gibson. v3: - More changes after feedback from David Gibson. v4: - Even more changes after feedback from D. Gibson v5: - Added corresponding patches for IPv6 Jon Maloy (4): tap: break out building of udp header from tap_udp4_send function udp: create and send ICMPv4 to local peer when applicable tap: break out building of udp header from tap_udp6_send function udp: create and send ICMPv6 to local peer when applicable tap.c | 84 ++++++++++++++++++++++++---------- tap.h | 15 +++++++ udp.c | 119 ++++++++++++++++++++++++++++++++++++++++++++----- udp_internal.h | 2 +- udp_vu.c | 4 +- 5 files changed, 187 insertions(+), 37 deletions(-) -- 2.48.1

2 7

[PATCH 0/2] RFC: More graceful handling of migration without passt-repair (UNTESTED)
by David Gibson 21 Feb '25

21 Feb '25

From Red Hat internal testing we've had some reports that if attempting to migrate without passt-repair, the failure mode is uglier than we'd like. The migration fails, which is somewhat expected, but we don't correctly roll things back on the source, so it breaks network there as well. Handle this more gracefully allowing the migration to proceed in this case, but allow TCP connections to break Unfortunately, I only spotted the ticket about this quite late in my day. I'm posting this now for visibility, but I ran out of time to test it. I'll do that tomorrow if someone doesn't pick it up before then. David Gibson (2): migrate, flow: Trivially succeed if migrating with no flows migrate, flow: Don't attempt to migrate TCP flows without passt-repair flow.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) -- 2.48.1

2 9

[PATCH 0/4] passt-repair improvements
by David Gibson 21 Feb '25

21 Feb '25

This series has a number of minor clean ups to passt-repair, plus one more significant change. The latter allows passt-repair to report partial failures, but does require a small change to its protocol. David Gibson (4): passt-repair: Add die() macro passt-repair: Consistently avoid strerror() passt-repair: Improve validation of anciliary data length passt-repair: Allow passt-repair to report partial failures passt-repair.c | 126 ++++++++++++++++++++++++------------------------- repair.c | 11 +++-- 2 files changed, 68 insertions(+), 69 deletions(-) -- 2.48.1

1 4