* Re: [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device
2025-09-24 6:28 [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device Houqi (Nick) Zuo
@ 2025-09-24 6:32 ` Jason Wang
2025-09-24 6:54 ` Cindy Lu
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Jason Wang @ 2025-09-24 6:32 UTC (permalink / raw)
To: Houqi (Nick) Zuo; +Cc: qemu-devel, Cindy Lu, Michael Tsirkin
On Wed, Sep 24, 2025 at 2:29 PM Houqi (Nick) Zuo <hzuo@redhat.com> wrote:
>
> This patch addresses a scenario where QEMU would abort with a core dump
> when a tap device created by QEMU is manually deleted from the host while
> the guest is running.
>
> The specific negative test case is:
> 1. Start QEMU with a tap device (created by QEMU)
> 2. Manually delete the tap device on the host
> 3. Execute shutdown in the guest
> 4. QEMU attempts to clean up the tap device but finds the file descriptor
> in a bad state, leading to abort and core dump
>
> The patch introduces a tap device file descriptor validity check using
> the TUNGETIFF ioctl to detect when the underlying tap device has been
> removed. When detected, the operations are skipped gracefully instead
> of proceeding with invalid file descriptors that cause ioctl failures.
>
> The validity check is integrated into:
> - qemu_set_vnet_hdr_len() in net/net.c
> - qemu_set_offload() in net/net.c
>
> This ensures that when the tap device is no longer valid, these functions
> return early without attempting operations that would fail and trigger
> aborts,
Let's just remove the abort then?
> thus achieving the expected behavior of error reporting without
> crashing.
>
> (gdb) bt full
> #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
> tid = <optimized out>
> ret = 0
> pd = <optimized out>
> old_mask = {__val = {10}}
> ret = <optimized out>
> #1 0x00007f1710b6bff3 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
> #2 0x00007f1710b15f56 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
> ret = <optimized out>
> #3 0x00007f1710afd8fa in __GI_abort () at abort.c:79
> save_stage = 1
> act = {__sigaction_handler = {sa_handler = 0x20, sa_sigaction = 0x20}, sa_mask = {__val = {16929458408262392576, 18446744073709550848, 139737042419943, 139737042419943, 0, 94049703655600, 139737042419943, 139737042670528, 18446744073709550328, 77, 139705603579344, 18446744073709551615, 139737041472378, 139705595179568, 16929458408262392576, 94049679794864}}, sa_flags = 281695456, sa_restorer = 0xa}
> #4 0x000055899a71de58 in tap_fd_set_vnet_hdr_len (fd=<optimized out>, len=10) at ../net/tap-linux.c:204
> #5 tap_set_vnet_hdr_len (nc=<optimized out>, len=10) at ../net/tap.c:269
> s = <optimized out>
> #6 0x000055899a8be67f in qemu_set_vnet_hdr_len (nc=0x2956, len=10588) at ../net/net.c:573
> #7 virtio_net_set_mrg_rx_bufs (n=0x5589a72cfa10, mergeable_rx_bufs=<optimized out>, version_1=<error reading variable: Incompatible types on DWARF stack>, hash_report=<optimized out>) at ../hw/net/virtio-net.c:664
> i = 0
> nc = 0x5589a730ab28
> #8 virtio_net_set_features (vdev=0x5589a72cfa10, features=0) at ../hw/net/virtio-net.c:897
> n = 0x5589a72cfa10
> err = 0x0
> i = 0
> #9 0x000055899a8e4eaa in virtio_set_features_nocheck (vdev=0x5589a72cfa10, val=0) at ../hw/virtio/virtio.c:3079
> k = <optimized out>
> bad = <optimized out>
> #10 virtio_reset (opaque=0x5589a72cfa10) at ../hw/virtio/virtio.c:3184
> vdev = 0x5589a72cfa10
> k = 0x5589a5c162b0
> i = 0
> #11 0x000055899a630d2b in virtio_bus_reset (bus=0x5589a72cf990) at ../hw/virtio/virtio-bus.c:109
> vdev = <optimized out>
> #12 virtio_pci_reset (qdev=0x5589a72c7470) at ../hw/virtio/virtio-pci.c:2311
> proxy = 0x5589a72c7470
> i = 0
> bus = 0x5589a72cf990
> #13 0x000055899a686ded in memory_region_write_accessor (mr=<optimized out>, addr=<optimized out>, value=<optimized out>, size=<optimized out>, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../system/memory.c:490
> tmp = <optimized out>
> #14 0x000055899a686cbc in access_with_adjusted_size (addr=20, value=0x7f0fbedfde00, size=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x55899a686d30 <memory_region_write_accessor>, mr=0x5589a72c8040, attrs=...) at ../system/memory.c:566
> print_once_ = false
> access_mask = 255
> access_size = 1
> i = 0
> r = 0
> reentrancy_guard_applied = <optimized out>
> #15 0x000055899a686ac5 in memory_region_dispatch_write (mr=<optimized out>, addr=20, data=<optimized out>, op=<optimized out>, attrs=...) at ../system/memory.c:1545
> size = <optimized out>
> #16 0x000055899a69f7da in flatview_write_continue_step (attrs=..., buf=0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>, len=<optimized out>, mr_addr=20, l=0x7f0fbedfde28, mr=0x5589a72c8040) at ../system/physmem.c:2972
> val = 6
> result = 0
> release_lock = <optimized out>
> #17 0x000055899a697c15 in flatview_write_continue (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., ptr=0x7f1711da6028, len=1, mr_addr=6, l=1, mr=0x0) at ../system/physmem.c:3002
> result = 0
> buf = 0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>
> #18 flatview_write (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3033
> --Type <RET> for more, q to quit, c to continue without paging--
> l = <optimized out>
> mr_addr = 6
> mr = 0x0
> #19 0x000055899a697a91 in address_space_write (as=0x55899bceeba0 <address_space_memory>, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3153
> _rcu_read_auto = 0x1
> result = 0
> fv = 0x2956
> #20 0x000055899a91159b in address_space_rw (addr=10588, attrs=..., buf=0x7f1711da6028, len=0, as=<optimized out>, is_write=<optimized out>) at ../system/physmem.c:3163
> #21 kvm_cpu_exec (cpu=0x5589a5d68b40) at ../accel/kvm/kvm-all.c:3255
> attrs = {secure = 0, space = 0, user = 0, memory = 0, debug = 0, requester_id = 0, pid = 0, address_type = 0, unspecified = false, _reserved1 = 0 '\000', _reserved2 = 0}
> run = 0x7f1711da6000
> ret = <optimized out>
> run_ret = <optimized out>
> #22 0x000055899a9189ca in kvm_vcpu_thread_fn (arg=0x5589a5d68b40) at ../accel/kvm/kvm-accel-ops.c:51
> r = <optimized out>
> cpu = <optimized out>
> #23 0x000055899aba817a in qemu_thread_start (args=0x5589a5d72580) at ../util/qemu-thread-posix.c:393
> __clframe = {__cancel_routine = <optimized out>, __cancel_arg = 0x0, __do_it = 1, __cancel_type = <optimized out>}
> qemu_thread_args = 0x5589a5d72580
> start_routine = 0x55899a918850 <kvm_vcpu_thread_fn>
> arg = 0x5589a5d68b40
> r = 0x0
> #24 0x00007f1710b6a128 in start_thread (arg=<optimized out>) at pthread_create.c:448
> ret = <optimized out>
> pd = <optimized out>
> out = <optimized out>
> unwind_buf = {cancel_jmp_buf = {{jmp_buf = {32, 8894544057743421332, -1288, 0, 140726164742416, 140726164742679, -8831356496486092908, -8844535456800460908}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
> not_first_call = <optimized out>
> #25 0x00007f1710bda924 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
>
> Signed-off-by: Houqi (Nick) Zuo <hzuo@redhat.com>
> ---
Thanks
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device
2025-09-24 6:28 [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device Houqi (Nick) Zuo
2025-09-24 6:32 ` Jason Wang
@ 2025-09-24 6:54 ` Cindy Lu
2025-09-24 7:04 ` Daniel P. Berrangé
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Cindy Lu @ 2025-09-24 6:54 UTC (permalink / raw)
To: Houqi (Nick) Zuo; +Cc: qemu-devel, Jason Wang, Michael Tsirkin
On Wed, Sep 24, 2025 at 2:28 PM Houqi (Nick) Zuo <hzuo@redhat.com> wrote:
>
> This patch addresses a scenario where QEMU would abort with a core dump
> when a tap device created by QEMU is manually deleted from the host while
> the guest is running.
>
> The specific negative test case is:
> 1. Start QEMU with a tap device (created by QEMU)
> 2. Manually delete the tap device on the host
> 3. Execute shutdown in the guest
> 4. QEMU attempts to clean up the tap device but finds the file descriptor
> in a bad state, leading to abort and core dump
>
> The patch introduces a tap device file descriptor validity check using
> the TUNGETIFF ioctl to detect when the underlying tap device has been
> removed. When detected, the operations are skipped gracefully instead
> of proceeding with invalid file descriptors that cause ioctl failures.
>
> The validity check is integrated into:
> - qemu_set_vnet_hdr_len() in net/net.c
> - qemu_set_offload() in net/net.c
>
> This ensures that when the tap device is no longer valid, these functions
> return early without attempting operations that would fail and trigger
> aborts, thus achieving the expected behavior of error reporting without
> crashing.
>
> (gdb) bt full
> #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
> tid = <optimized out>
> ret = 0
> pd = <optimized out>
> old_mask = {__val = {10}}
> ret = <optimized out>
> #1 0x00007f1710b6bff3 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
> #2 0x00007f1710b15f56 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
> ret = <optimized out>
> #3 0x00007f1710afd8fa in __GI_abort () at abort.c:79
> save_stage = 1
> act = {__sigaction_handler = {sa_handler = 0x20, sa_sigaction = 0x20}, sa_mask = {__val = {16929458408262392576, 18446744073709550848, 139737042419943, 139737042419943, 0, 94049703655600, 139737042419943, 139737042670528, 18446744073709550328, 77, 139705603579344, 18446744073709551615, 139737041472378, 139705595179568, 16929458408262392576, 94049679794864}}, sa_flags = 281695456, sa_restorer = 0xa}
> #4 0x000055899a71de58 in tap_fd_set_vnet_hdr_len (fd=<optimized out>, len=10) at ../net/tap-linux.c:204
> #5 tap_set_vnet_hdr_len (nc=<optimized out>, len=10) at ../net/tap.c:269
> s = <optimized out>
> #6 0x000055899a8be67f in qemu_set_vnet_hdr_len (nc=0x2956, len=10588) at ../net/net.c:573
> #7 virtio_net_set_mrg_rx_bufs (n=0x5589a72cfa10, mergeable_rx_bufs=<optimized out>, version_1=<error reading variable: Incompatible types on DWARF stack>, hash_report=<optimized out>) at ../hw/net/virtio-net.c:664
> i = 0
> nc = 0x5589a730ab28
> #8 virtio_net_set_features (vdev=0x5589a72cfa10, features=0) at ../hw/net/virtio-net.c:897
> n = 0x5589a72cfa10
> err = 0x0
> i = 0
> #9 0x000055899a8e4eaa in virtio_set_features_nocheck (vdev=0x5589a72cfa10, val=0) at ../hw/virtio/virtio.c:3079
> k = <optimized out>
> bad = <optimized out>
> #10 virtio_reset (opaque=0x5589a72cfa10) at ../hw/virtio/virtio.c:3184
> vdev = 0x5589a72cfa10
> k = 0x5589a5c162b0
> i = 0
> #11 0x000055899a630d2b in virtio_bus_reset (bus=0x5589a72cf990) at ../hw/virtio/virtio-bus.c:109
> vdev = <optimized out>
> #12 virtio_pci_reset (qdev=0x5589a72c7470) at ../hw/virtio/virtio-pci.c:2311
> proxy = 0x5589a72c7470
> i = 0
> bus = 0x5589a72cf990
> #13 0x000055899a686ded in memory_region_write_accessor (mr=<optimized out>, addr=<optimized out>, value=<optimized out>, size=<optimized out>, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../system/memory.c:490
> tmp = <optimized out>
> #14 0x000055899a686cbc in access_with_adjusted_size (addr=20, value=0x7f0fbedfde00, size=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x55899a686d30 <memory_region_write_accessor>, mr=0x5589a72c8040, attrs=...) at ../system/memory.c:566
> print_once_ = false
> access_mask = 255
> access_size = 1
> i = 0
> r = 0
> reentrancy_guard_applied = <optimized out>
> #15 0x000055899a686ac5 in memory_region_dispatch_write (mr=<optimized out>, addr=20, data=<optimized out>, op=<optimized out>, attrs=...) at ../system/memory.c:1545
> size = <optimized out>
> #16 0x000055899a69f7da in flatview_write_continue_step (attrs=..., buf=0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>, len=<optimized out>, mr_addr=20, l=0x7f0fbedfde28, mr=0x5589a72c8040) at ../system/physmem.c:2972
> val = 6
> result = 0
> release_lock = <optimized out>
> #17 0x000055899a697c15 in flatview_write_continue (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., ptr=0x7f1711da6028, len=1, mr_addr=6, l=1, mr=0x0) at ../system/physmem.c:3002
> result = 0
> buf = 0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>
> #18 flatview_write (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3033
> --Type <RET> for more, q to quit, c to continue without paging--
> l = <optimized out>
> mr_addr = 6
> mr = 0x0
> #19 0x000055899a697a91 in address_space_write (as=0x55899bceeba0 <address_space_memory>, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3153
> _rcu_read_auto = 0x1
> result = 0
> fv = 0x2956
> #20 0x000055899a91159b in address_space_rw (addr=10588, attrs=..., buf=0x7f1711da6028, len=0, as=<optimized out>, is_write=<optimized out>) at ../system/physmem.c:3163
> #21 kvm_cpu_exec (cpu=0x5589a5d68b40) at ../accel/kvm/kvm-all.c:3255
> attrs = {secure = 0, space = 0, user = 0, memory = 0, debug = 0, requester_id = 0, pid = 0, address_type = 0, unspecified = false, _reserved1 = 0 '\000', _reserved2 = 0}
> run = 0x7f1711da6000
> ret = <optimized out>
> run_ret = <optimized out>
> #22 0x000055899a9189ca in kvm_vcpu_thread_fn (arg=0x5589a5d68b40) at ../accel/kvm/kvm-accel-ops.c:51
> r = <optimized out>
> cpu = <optimized out>
> #23 0x000055899aba817a in qemu_thread_start (args=0x5589a5d72580) at ../util/qemu-thread-posix.c:393
> __clframe = {__cancel_routine = <optimized out>, __cancel_arg = 0x0, __do_it = 1, __cancel_type = <optimized out>}
> qemu_thread_args = 0x5589a5d72580
> start_routine = 0x55899a918850 <kvm_vcpu_thread_fn>
> arg = 0x5589a5d68b40
> r = 0x0
> #24 0x00007f1710b6a128 in start_thread (arg=<optimized out>) at pthread_create.c:448
> ret = <optimized out>
> pd = <optimized out>
> out = <optimized out>
> unwind_buf = {cancel_jmp_buf = {{jmp_buf = {32, 8894544057743421332, -1288, 0, 140726164742416, 140726164742679, -8831356496486092908, -8844535456800460908}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
> not_first_call = <optimized out>
> #25 0x00007f1710bda924 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
>
Please also add Fixes: ***commit id ***** in the commit log
> Signed-off-by: Houqi (Nick) Zuo <hzuo@redhat.com>
> ---
> include/net/net.h | 2 ++
> net/net.c | 7 +++++--
> net/tap-linux.c | 11 +++++++++++
> net/tap.c | 9 +++++++++
> net/tap_int.h | 1 +
> 5 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 84ee18e0f9..9e435f3275 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -69,6 +69,7 @@ typedef void (NetAnnounce)(NetClientState *);
> typedef bool (SetSteeringEBPF)(NetClientState *, int);
> typedef bool (NetCheckPeerType)(NetClientState *, ObjectClass *, Error **);
> typedef struct vhost_net *(GetVHostNet)(NetClientState *nc);
> +typedef int (QueryValidity)(NetClientState *nc);
>
> typedef struct NetClientInfo {
> NetClientDriver type;
> @@ -96,6 +97,7 @@ typedef struct NetClientInfo {
> SetSteeringEBPF *set_steering_ebpf;
> NetCheckPeerType *check_peer_type;
> GetVHostNet *get_vhost_net;
> + QueryValidity *query_validity;
> } NetClientInfo;
>
> struct NetClientState {
> diff --git a/net/net.c b/net/net.c
> index da275db86e..c0750fd0b9 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -57,6 +57,7 @@
> #include "qapi/string-output-visitor.h"
> #include "qapi/qobject-input-visitor.h"
> #include "standard-headers/linux/virtio_net.h"
> +#include "qemu/log.h"
>
> /* Net bridge is currently not supported for W32. */
> #if !defined(_WIN32)
> @@ -543,7 +544,8 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len)
> void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
> int ecn, int ufo, int uso4, int uso6)
> {
> - if (!nc || !nc->info->set_offload) {
> + if (!nc || !nc->info->set_offload ||
> + (nc->info->query_validity && nc->info->query_validity(nc) != 1)) {
> return;
> }
>
> @@ -561,7 +563,8 @@ int qemu_get_vnet_hdr_len(NetClientState *nc)
>
> void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
> {
> - if (!nc || !nc->info->set_vnet_hdr_len) {
> + if (!nc || !nc->info->set_vnet_hdr_len ||
> + (nc->info->query_validity && nc->info->query_validity(nc) != 1)) {
> return;
> }
>
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index e832810665..2911c66149 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -346,3 +346,14 @@ int tap_fd_set_steering_ebpf(int fd, int prog_fd)
>
> return 0;
> }
> +
> +int tap_fd_query_validity(int fd)
> +{
> + struct ifreq ifr;
> +
> + if (ioctl(fd, TUNGETIFF, &ifr) != 0) {
> + error_report("The tap device fd: %d is NOT valid.", fd);
> + return -1;
> + }
> + return 1;
> +}
> diff --git a/net/tap.c b/net/tap.c
> index f37133e301..a9af3c7279 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -364,6 +364,14 @@ static VHostNetState *tap_get_vhost_net(NetClientState *nc)
> return s->vhost_net;
> }
>
> +static int tap_query_validity(NetClientState *nc)
> +{
> + TAPState *s = DO_UPCAST(TAPState, nc, nc);
> + assert(nc->info->type == NET_CLIENT_DRIVER_TAP);
> +
> + return tap_fd_query_validity(s->fd);
> +}
> +
> /* fd support */
>
> static NetClientInfo net_tap_info = {
> @@ -383,6 +391,7 @@ static NetClientInfo net_tap_info = {
> .set_vnet_be = tap_set_vnet_be,
> .set_steering_ebpf = tap_set_steering_ebpf,
> .get_vhost_net = tap_get_vhost_net,
> + .query_validity = tap_query_validity,
> };
>
> static TAPState *net_tap_fd_init(NetClientState *peer,
> diff --git a/net/tap_int.h b/net/tap_int.h
> index 8857ff299d..5deb380201 100644
> --- a/net/tap_int.h
> +++ b/net/tap_int.h
> @@ -46,5 +46,6 @@ int tap_fd_enable(int fd);
> int tap_fd_disable(int fd);
> int tap_fd_get_ifname(int fd, char *ifname);
> int tap_fd_set_steering_ebpf(int fd, int prog_fd);
> +int tap_fd_query_validity(int fd);
>
> #endif /* NET_TAP_INT_H */
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device
2025-09-24 6:28 [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device Houqi (Nick) Zuo
2025-09-24 6:32 ` Jason Wang
2025-09-24 6:54 ` Cindy Lu
@ 2025-09-24 7:04 ` Daniel P. Berrangé
2025-09-24 8:14 ` [PATCH v2] net/tap-linux.c: avoid abort when setting invalid fd Houqi (Nick) Zuo
2025-09-24 8:50 ` [PATCH v3] " Houqi (Nick) Zuo
4 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2025-09-24 7:04 UTC (permalink / raw)
To: Houqi (Nick) Zuo; +Cc: qemu-devel, Jason Wang, Cindy Lu, Michael Tsirkin
On Wed, Sep 24, 2025 at 02:28:31PM +0800, Houqi (Nick) Zuo wrote:
> This patch addresses a scenario where QEMU would abort with a core dump
> when a tap device created by QEMU is manually deleted from the host while
> the guest is running.
>
> The specific negative test case is:
> 1. Start QEMU with a tap device (created by QEMU)
> 2. Manually delete the tap device on the host
> 3. Execute shutdown in the guest
> 4. QEMU attempts to clean up the tap device but finds the file descriptor
> in a bad state, leading to abort and core dump
>
> The patch introduces a tap device file descriptor validity check using
> the TUNGETIFF ioctl to detect when the underlying tap device has been
> removed. When detected, the operations are skipped gracefully instead
> of proceeding with invalid file descriptors that cause ioctl failures.
>
> The validity check is integrated into:
> - qemu_set_vnet_hdr_len() in net/net.c
> - qemu_set_offload() in net/net.c
>
> This ensures that when the tap device is no longer valid, these functions
> return early without attempting operations that would fail and trigger
> aborts, thus achieving the expected behavior of error reporting without
> diff --git a/net/net.c b/net/net.c
> index da275db86e..c0750fd0b9 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -57,6 +57,7 @@
> #include "qapi/string-output-visitor.h"
> #include "qapi/qobject-input-visitor.h"
> #include "standard-headers/linux/virtio_net.h"
> +#include "qemu/log.h"
>
> /* Net bridge is currently not supported for W32. */
> #if !defined(_WIN32)
> @@ -543,7 +544,8 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len)
> void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
> int ecn, int ufo, int uso4, int uso6)
> {
> - if (!nc || !nc->info->set_offload) {
> + if (!nc || !nc->info->set_offload ||
> + (nc->info->query_validity && nc->info->query_validity(nc) != 1)) {
> return;
> }
>
> @@ -561,7 +563,8 @@ int qemu_get_vnet_hdr_len(NetClientState *nc)
>
> void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
> {
> - if (!nc || !nc->info->set_vnet_hdr_len) {
> + if (!nc || !nc->info->set_vnet_hdr_len ||
> + (nc->info->query_validity && nc->info->query_validity(nc) != 1)) {
> return;
> }
>
These checks leave open a race condition where the tap can be
deleted in between checking the tap and trying the operation.
Drop the check and just make the later operation not abort
on failure.
There are other methods in this file which will abort on
failure too tap that need addressing, because if setting
len failure is ignored it'll go on to call qemu_set_vnet_be
/ qemu_set_vnet_le eventually which will abort for the same
reason.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] net/tap-linux.c: avoid abort when setting invalid fd
2025-09-24 6:28 [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device Houqi (Nick) Zuo
` (2 preceding siblings ...)
2025-09-24 7:04 ` Daniel P. Berrangé
@ 2025-09-24 8:14 ` Houqi (Nick) Zuo
2025-09-24 8:26 ` Daniel P. Berrangé
2025-09-24 8:50 ` [PATCH v3] " Houqi (Nick) Zuo
4 siblings, 1 reply; 8+ messages in thread
From: Houqi (Nick) Zuo @ 2025-09-24 8:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Jason Wang, Cindy Lu, Michael Tsirkin
When QEMU creates a tap device automatically and the tap device is
manually removed from the host while the guest is running, the tap
device file descriptor becomes invalid. Later, when the guest executes
shutdown, the tap_fd_set_vnet_hdr_len() function may be called and
abort QEMU with a core dump when attempting to use the invalid fd.
This patch removes many abort() calls in this file. If the fd is found
to be in a bad state (e.g., EBADFD or ENODEV), the related function
will print an error message.
The expected behavior for this negative test case is that QEMU should
report an error but continue running rather than aborting.
Testing:
- Start QEMU with automatically created tap device
- Manually remove the tap device on the host
- Execute shutdown in the guest
- Verify QEMU reports an error but does not abort
(gdb) bt full
#0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
tid = <optimized out>
ret = 0
pd = <optimized out>
old_mask = {__val = {10}}
ret = <optimized out>
#1 0x00007f1710b6bff3 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
#2 0x00007f1710b15f56 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
ret = <optimized out>
#3 0x00007f1710afd8fa in __GI_abort () at abort.c:79
save_stage = 1
act = {__sigaction_handler = {sa_handler = 0x20, sa_sigaction = 0x20}, sa_mask = {__val = {16929458408262392576, 18446744073709550848, 139737042419943, 139737042419943, 0, 94049703655600, 139737042419943, 139737042670528, 18446744073709550328, 77, 139705603579344, 18446744073709551615, 139737041472378, 139705595179568, 16929458408262392576, 94049679794864}}, sa_flags = 281695456, sa_restorer = 0xa}
#4 0x000055899a71de58 in tap_fd_set_vnet_hdr_len (fd=<optimized out>, len=10) at ../net/tap-linux.c:204
#5 tap_set_vnet_hdr_len (nc=<optimized out>, len=10) at ../net/tap.c:269
s = <optimized out>
#6 0x000055899a8be67f in qemu_set_vnet_hdr_len (nc=0x2956, len=10588) at ../net/net.c:573
#7 virtio_net_set_mrg_rx_bufs (n=0x5589a72cfa10, mergeable_rx_bufs=<optimized out>, version_1=<error reading variable: Incompatible types on DWARF stack>, hash_report=<optimized out>) at ../hw/net/virtio-net.c:664
i = 0
nc = 0x5589a730ab28
#8 virtio_net_set_features (vdev=0x5589a72cfa10, features=0) at ../hw/net/virtio-net.c:897
n = 0x5589a72cfa10
err = 0x0
i = 0
#9 0x000055899a8e4eaa in virtio_set_features_nocheck (vdev=0x5589a72cfa10, val=0) at ../hw/virtio/virtio.c:3079
k = <optimized out>
bad = <optimized out>
#10 virtio_reset (opaque=0x5589a72cfa10) at ../hw/virtio/virtio.c:3184
vdev = 0x5589a72cfa10
k = 0x5589a5c162b0
i = 0
#11 0x000055899a630d2b in virtio_bus_reset (bus=0x5589a72cf990) at ../hw/virtio/virtio-bus.c:109
vdev = <optimized out>
#12 virtio_pci_reset (qdev=0x5589a72c7470) at ../hw/virtio/virtio-pci.c:2311
proxy = 0x5589a72c7470
i = 0
bus = 0x5589a72cf990
#13 0x000055899a686ded in memory_region_write_accessor (mr=<optimized out>, addr=<optimized out>, value=<optimized out>, size=<optimized out>, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../system/memory.c:490
tmp = <optimized out>
#14 0x000055899a686cbc in access_with_adjusted_size (addr=20, value=0x7f0fbedfde00, size=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x55899a686d30 <memory_region_write_accessor>, mr=0x5589a72c8040, attrs=...) at ../system/memory.c:566
print_once_ = false
access_mask = 255
access_size = 1
i = 0
r = 0
reentrancy_guard_applied = <optimized out>
#15 0x000055899a686ac5 in memory_region_dispatch_write (mr=<optimized out>, addr=20, data=<optimized out>, op=<optimized out>, attrs=...) at ../system/memory.c:1545
size = <optimized out>
#16 0x000055899a69f7da in flatview_write_continue_step (attrs=..., buf=0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>, len=<optimized out>, mr_addr=20, l=0x7f0fbedfde28, mr=0x5589a72c8040) at ../system/physmem.c:2972
val = 6
result = 0
release_lock = <optimized out>
#17 0x000055899a697c15 in flatview_write_continue (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., ptr=0x7f1711da6028, len=1, mr_addr=6, l=1, mr=0x0) at ../system/physmem.c:3002
result = 0
buf = 0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>
#18 flatview_write (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3033
--Type <RET> for more, q to quit, c to continue without paging--
l = <optimized out>
mr_addr = 6
mr = 0x0
#19 0x000055899a697a91 in address_space_write (as=0x55899bceeba0 <address_space_memory>, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3153
_rcu_read_auto = 0x1
result = 0
fv = 0x2956
#20 0x000055899a91159b in address_space_rw (addr=10588, attrs=..., buf=0x7f1711da6028, len=0, as=<optimized out>, is_write=<optimized out>) at ../system/physmem.c:3163
#21 kvm_cpu_exec (cpu=0x5589a5d68b40) at ../accel/kvm/kvm-all.c:3255
attrs = {secure = 0, space = 0, user = 0, memory = 0, debug = 0, requester_id = 0, pid = 0, address_type = 0, unspecified = false, _reserved1 = 0 '\000', _reserved2 = 0}
run = 0x7f1711da6000
ret = <optimized out>
run_ret = <optimized out>
#22 0x000055899a9189ca in kvm_vcpu_thread_fn (arg=0x5589a5d68b40) at ../accel/kvm/kvm-accel-ops.c:51
r = <optimized out>
cpu = <optimized out>
#23 0x000055899aba817a in qemu_thread_start (args=0x5589a5d72580) at ../util/qemu-thread-posix.c:393
__clframe = {__cancel_routine = <optimized out>, __cancel_arg = 0x0, __do_it = 1, __cancel_type = <optimized out>}
qemu_thread_args = 0x5589a5d72580
start_routine = 0x55899a918850 <kvm_vcpu_thread_fn>
arg = 0x5589a5d68b40
r = 0x0
#24 0x00007f1710b6a128 in start_thread (arg=<optimized out>) at pthread_create.c:448
ret = <optimized out>
pd = <optimized out>
out = <optimized out>
unwind_buf = {cancel_jmp_buf = {{jmp_buf = {32, 8894544057743421332, -1288, 0, 140726164742416, 140726164742679, -8831356496486092908, -8844535456800460908}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
not_first_call = <optimized out>
#25 0x00007f1710bda924 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
Fixes: 0caed25cd171c611781589b5402161d27d57229c virtio: Call set_features during reset
Signed-off-by: Houqi (Nick) Zuo <hzuo@redhat.com>
---
net/tap-linux.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index e832810665..140305d406 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -206,7 +206,6 @@ void tap_fd_set_vnet_hdr_len(int fd, int len)
if (ioctl(fd, TUNSETVNETHDRSZ, &len) == -1) {
fprintf(stderr, "TUNSETVNETHDRSZ ioctl() failed: %s. Exiting.\n",
strerror(errno));
- abort();
}
}
@@ -224,7 +223,6 @@ int tap_fd_set_vnet_le(int fd, int is_le)
}
error_report("TUNSETVNETLE ioctl() failed: %s.", strerror(errno));
- abort();
}
int tap_fd_set_vnet_be(int fd, int is_be)
@@ -241,7 +239,6 @@ int tap_fd_set_vnet_be(int fd, int is_be)
}
error_report("TUNSETVNETBE ioctl() failed: %s.", strerror(errno));
- abort();
}
void tap_fd_set_offload(int fd, int csum, int tso4,
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] net/tap-linux.c: avoid abort when setting invalid fd
2025-09-24 8:14 ` [PATCH v2] net/tap-linux.c: avoid abort when setting invalid fd Houqi (Nick) Zuo
@ 2025-09-24 8:26 ` Daniel P. Berrangé
0 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2025-09-24 8:26 UTC (permalink / raw)
To: Houqi (Nick) Zuo; +Cc: qemu-devel, Jason Wang, Cindy Lu, Michael Tsirkin
On Wed, Sep 24, 2025 at 04:14:51PM +0800, Houqi (Nick) Zuo wrote:
> When QEMU creates a tap device automatically and the tap device is
> manually removed from the host while the guest is running, the tap
> device file descriptor becomes invalid. Later, when the guest executes
> shutdown, the tap_fd_set_vnet_hdr_len() function may be called and
> abort QEMU with a core dump when attempting to use the invalid fd.
>
> This patch removes many abort() calls in this file. If the fd is found
> to be in a bad state (e.g., EBADFD or ENODEV), the related function
> will print an error message.
>
> The expected behavior for this negative test case is that QEMU should
> report an error but continue running rather than aborting.
>
> Testing:
> - Start QEMU with automatically created tap device
> - Manually remove the tap device on the host
> - Execute shutdown in the guest
> - Verify QEMU reports an error but does not abort
>
> (gdb) bt full
> #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
> tid = <optimized out>
> ret = 0
> pd = <optimized out>
> old_mask = {__val = {10}}
> ret = <optimized out>
> #1 0x00007f1710b6bff3 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
> #2 0x00007f1710b15f56 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
> ret = <optimized out>
> #3 0x00007f1710afd8fa in __GI_abort () at abort.c:79
> save_stage = 1
> act = {__sigaction_handler = {sa_handler = 0x20, sa_sigaction = 0x20}, sa_mask = {__val = {16929458408262392576, 18446744073709550848, 139737042419943, 139737042419943, 0, 94049703655600, 139737042419943, 139737042670528, 18446744073709550328, 77, 139705603579344, 18446744073709551615, 139737041472378, 139705595179568, 16929458408262392576, 94049679794864}}, sa_flags = 281695456, sa_restorer = 0xa}
> #4 0x000055899a71de58 in tap_fd_set_vnet_hdr_len (fd=<optimized out>, len=10) at ../net/tap-linux.c:204
> #5 tap_set_vnet_hdr_len (nc=<optimized out>, len=10) at ../net/tap.c:269
> s = <optimized out>
> #6 0x000055899a8be67f in qemu_set_vnet_hdr_len (nc=0x2956, len=10588) at ../net/net.c:573
> #7 virtio_net_set_mrg_rx_bufs (n=0x5589a72cfa10, mergeable_rx_bufs=<optimized out>, version_1=<error reading variable: Incompatible types on DWARF stack>, hash_report=<optimized out>) at ../hw/net/virtio-net.c:664
> i = 0
> nc = 0x5589a730ab28
> #8 virtio_net_set_features (vdev=0x5589a72cfa10, features=0) at ../hw/net/virtio-net.c:897
> n = 0x5589a72cfa10
> err = 0x0
> i = 0
> #9 0x000055899a8e4eaa in virtio_set_features_nocheck (vdev=0x5589a72cfa10, val=0) at ../hw/virtio/virtio.c:3079
> k = <optimized out>
> bad = <optimized out>
> #10 virtio_reset (opaque=0x5589a72cfa10) at ../hw/virtio/virtio.c:3184
> vdev = 0x5589a72cfa10
> k = 0x5589a5c162b0
> i = 0
> #11 0x000055899a630d2b in virtio_bus_reset (bus=0x5589a72cf990) at ../hw/virtio/virtio-bus.c:109
> vdev = <optimized out>
> #12 virtio_pci_reset (qdev=0x5589a72c7470) at ../hw/virtio/virtio-pci.c:2311
> proxy = 0x5589a72c7470
> i = 0
> bus = 0x5589a72cf990
> #13 0x000055899a686ded in memory_region_write_accessor (mr=<optimized out>, addr=<optimized out>, value=<optimized out>, size=<optimized out>, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../system/memory.c:490
> tmp = <optimized out>
> #14 0x000055899a686cbc in access_with_adjusted_size (addr=20, value=0x7f0fbedfde00, size=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x55899a686d30 <memory_region_write_accessor>, mr=0x5589a72c8040, attrs=...) at ../system/memory.c:566
> print_once_ = false
> access_mask = 255
> access_size = 1
> i = 0
> r = 0
> reentrancy_guard_applied = <optimized out>
> #15 0x000055899a686ac5 in memory_region_dispatch_write (mr=<optimized out>, addr=20, data=<optimized out>, op=<optimized out>, attrs=...) at ../system/memory.c:1545
> size = <optimized out>
> #16 0x000055899a69f7da in flatview_write_continue_step (attrs=..., buf=0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>, len=<optimized out>, mr_addr=20, l=0x7f0fbedfde28, mr=0x5589a72c8040) at ../system/physmem.c:2972
> val = 6
> result = 0
> release_lock = <optimized out>
> #17 0x000055899a697c15 in flatview_write_continue (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., ptr=0x7f1711da6028, len=1, mr_addr=6, l=1, mr=0x0) at ../system/physmem.c:3002
> result = 0
> buf = 0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>
> #18 flatview_write (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3033
> --Type <RET> for more, q to quit, c to continue without paging--
> l = <optimized out>
> mr_addr = 6
> mr = 0x0
> #19 0x000055899a697a91 in address_space_write (as=0x55899bceeba0 <address_space_memory>, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3153
> _rcu_read_auto = 0x1
> result = 0
> fv = 0x2956
> #20 0x000055899a91159b in address_space_rw (addr=10588, attrs=..., buf=0x7f1711da6028, len=0, as=<optimized out>, is_write=<optimized out>) at ../system/physmem.c:3163
> #21 kvm_cpu_exec (cpu=0x5589a5d68b40) at ../accel/kvm/kvm-all.c:3255
> attrs = {secure = 0, space = 0, user = 0, memory = 0, debug = 0, requester_id = 0, pid = 0, address_type = 0, unspecified = false, _reserved1 = 0 '\000', _reserved2 = 0}
> run = 0x7f1711da6000
> ret = <optimized out>
> run_ret = <optimized out>
> #22 0x000055899a9189ca in kvm_vcpu_thread_fn (arg=0x5589a5d68b40) at ../accel/kvm/kvm-accel-ops.c:51
> r = <optimized out>
> cpu = <optimized out>
> #23 0x000055899aba817a in qemu_thread_start (args=0x5589a5d72580) at ../util/qemu-thread-posix.c:393
> __clframe = {__cancel_routine = <optimized out>, __cancel_arg = 0x0, __do_it = 1, __cancel_type = <optimized out>}
> qemu_thread_args = 0x5589a5d72580
> start_routine = 0x55899a918850 <kvm_vcpu_thread_fn>
> arg = 0x5589a5d68b40
> r = 0x0
> #24 0x00007f1710b6a128 in start_thread (arg=<optimized out>) at pthread_create.c:448
> ret = <optimized out>
> pd = <optimized out>
> out = <optimized out>
> unwind_buf = {cancel_jmp_buf = {{jmp_buf = {32, 8894544057743421332, -1288, 0, 140726164742416, 140726164742679, -8831356496486092908, -8844535456800460908}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
> not_first_call = <optimized out>
> #25 0x00007f1710bda924 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
>
> Fixes: 0caed25cd171c611781589b5402161d27d57229c virtio: Call set_features during reset
>
> Signed-off-by: Houqi (Nick) Zuo <hzuo@redhat.com>
> ---
> net/tap-linux.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index e832810665..140305d406 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -206,7 +206,6 @@ void tap_fd_set_vnet_hdr_len(int fd, int len)
> if (ioctl(fd, TUNSETVNETHDRSZ, &len) == -1) {
> fprintf(stderr, "TUNSETVNETHDRSZ ioctl() failed: %s. Exiting.\n",
> strerror(errno));
> - abort();
> }
> }
>
> @@ -224,7 +223,6 @@ int tap_fd_set_vnet_le(int fd, int is_le)
> }
>
> error_report("TUNSETVNETLE ioctl() failed: %s.", strerror(errno));
> - abort();
> }
>
> int tap_fd_set_vnet_be(int fd, int is_be)
> @@ -241,7 +239,6 @@ int tap_fd_set_vnet_be(int fd, int is_be)
> }
>
> error_report("TUNSETVNETBE ioctl() failed: %s.", strerror(errno));
> - abort();
> }
I rather doubt this will even compile, because this method has 'int'
return type without any 'return' statement present.
>
> void tap_fd_set_offload(int fd, int csum, int tso4,
> --
> 2.47.3
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] net/tap-linux.c: avoid abort when setting invalid fd
2025-09-24 6:28 [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device Houqi (Nick) Zuo
` (3 preceding siblings ...)
2025-09-24 8:14 ` [PATCH v2] net/tap-linux.c: avoid abort when setting invalid fd Houqi (Nick) Zuo
@ 2025-09-24 8:50 ` Houqi (Nick) Zuo
2025-09-25 1:01 ` Cindy Lu
4 siblings, 1 reply; 8+ messages in thread
From: Houqi (Nick) Zuo @ 2025-09-24 8:50 UTC (permalink / raw)
To: qemu-devel; +Cc: Jason Wang, Cindy Lu, Michael Tsirkin
When QEMU creates a tap device automatically and the tap device is
manually removed from the host while the guest is running, the tap
device file descriptor becomes invalid. Later, when the guest executes
shutdown, the tap_fd_set_vnet_hdr_len() function may be called and
abort QEMU with a core dump when attempting to use the invalid fd.
This patch removes many abort() calls in this file. If the fd is found
to be in a bad state (e.g., EBADFD or ENODEV), the related function
will print an error message.
The expected behavior for this negative test case is that QEMU should
report an error but continue running rather than aborting.
Testing:
- Start QEMU with automatically created tap device
- Manually remove the tap device on the host
- Execute shutdown in the guest
- Verify QEMU reports an error but does not abort
(gdb) bt full
#0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
tid = <optimized out>
ret = 0
pd = <optimized out>
old_mask = {__val = {10}}
ret = <optimized out>
#1 0x00007f1710b6bff3 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
#2 0x00007f1710b15f56 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
ret = <optimized out>
#3 0x00007f1710afd8fa in __GI_abort () at abort.c:79
save_stage = 1
act = {__sigaction_handler = {sa_handler = 0x20, sa_sigaction = 0x20}, sa_mask = {__val = {16929458408262392576, 18446744073709550848, 139737042419943, 139737042419943, 0, 94049703655600, 139737042419943, 139737042670528, 18446744073709550328, 77, 139705603579344, 18446744073709551615, 139737041472378, 139705595179568, 16929458408262392576, 94049679794864}}, sa_flags = 281695456, sa_restorer = 0xa}
#4 0x000055899a71de58 in tap_fd_set_vnet_hdr_len (fd=<optimized out>, len=10) at ../net/tap-linux.c:204
#5 tap_set_vnet_hdr_len (nc=<optimized out>, len=10) at ../net/tap.c:269
s = <optimized out>
#6 0x000055899a8be67f in qemu_set_vnet_hdr_len (nc=0x2956, len=10588) at ../net/net.c:573
#7 virtio_net_set_mrg_rx_bufs (n=0x5589a72cfa10, mergeable_rx_bufs=<optimized out>, version_1=<error reading variable: Incompatible types on DWARF stack>, hash_report=<optimized out>) at ../hw/net/virtio-net.c:664
i = 0
nc = 0x5589a730ab28
#8 virtio_net_set_features (vdev=0x5589a72cfa10, features=0) at ../hw/net/virtio-net.c:897
n = 0x5589a72cfa10
err = 0x0
i = 0
#9 0x000055899a8e4eaa in virtio_set_features_nocheck (vdev=0x5589a72cfa10, val=0) at ../hw/virtio/virtio.c:3079
k = <optimized out>
bad = <optimized out>
#10 virtio_reset (opaque=0x5589a72cfa10) at ../hw/virtio/virtio.c:3184
vdev = 0x5589a72cfa10
k = 0x5589a5c162b0
i = 0
#11 0x000055899a630d2b in virtio_bus_reset (bus=0x5589a72cf990) at ../hw/virtio/virtio-bus.c:109
vdev = <optimized out>
#12 virtio_pci_reset (qdev=0x5589a72c7470) at ../hw/virtio/virtio-pci.c:2311
proxy = 0x5589a72c7470
i = 0
bus = 0x5589a72cf990
#13 0x000055899a686ded in memory_region_write_accessor (mr=<optimized out>, addr=<optimized out>, value=<optimized out>, size=<optimized out>, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../system/memory.c:490
tmp = <optimized out>
#14 0x000055899a686cbc in access_with_adjusted_size (addr=20, value=0x7f0fbedfde00, size=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x55899a686d30 <memory_region_write_accessor>, mr=0x5589a72c8040, attrs=...) at ../system/memory.c:566
print_once_ = false
access_mask = 255
access_size = 1
i = 0
r = 0
reentrancy_guard_applied = <optimized out>
#15 0x000055899a686ac5 in memory_region_dispatch_write (mr=<optimized out>, addr=20, data=<optimized out>, op=<optimized out>, attrs=...) at ../system/memory.c:1545
size = <optimized out>
#16 0x000055899a69f7da in flatview_write_continue_step (attrs=..., buf=0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>, len=<optimized out>, mr_addr=20, l=0x7f0fbedfde28, mr=0x5589a72c8040) at ../system/physmem.c:2972
val = 6
result = 0
release_lock = <optimized out>
#17 0x000055899a697c15 in flatview_write_continue (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., ptr=0x7f1711da6028, len=1, mr_addr=6, l=1, mr=0x0) at ../system/physmem.c:3002
result = 0
buf = 0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>
#18 flatview_write (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3033
--Type <RET> for more, q to quit, c to continue without paging--
l = <optimized out>
mr_addr = 6
mr = 0x0
#19 0x000055899a697a91 in address_space_write (as=0x55899bceeba0 <address_space_memory>, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3153
_rcu_read_auto = 0x1
result = 0
fv = 0x2956
#20 0x000055899a91159b in address_space_rw (addr=10588, attrs=..., buf=0x7f1711da6028, len=0, as=<optimized out>, is_write=<optimized out>) at ../system/physmem.c:3163
#21 kvm_cpu_exec (cpu=0x5589a5d68b40) at ../accel/kvm/kvm-all.c:3255
attrs = {secure = 0, space = 0, user = 0, memory = 0, debug = 0, requester_id = 0, pid = 0, address_type = 0, unspecified = false, _reserved1 = 0 '\000', _reserved2 = 0}
run = 0x7f1711da6000
ret = <optimized out>
run_ret = <optimized out>
#22 0x000055899a9189ca in kvm_vcpu_thread_fn (arg=0x5589a5d68b40) at ../accel/kvm/kvm-accel-ops.c:51
r = <optimized out>
cpu = <optimized out>
#23 0x000055899aba817a in qemu_thread_start (args=0x5589a5d72580) at ../util/qemu-thread-posix.c:393
__clframe = {__cancel_routine = <optimized out>, __cancel_arg = 0x0, __do_it = 1, __cancel_type = <optimized out>}
qemu_thread_args = 0x5589a5d72580
start_routine = 0x55899a918850 <kvm_vcpu_thread_fn>
arg = 0x5589a5d68b40
r = 0x0
#24 0x00007f1710b6a128 in start_thread (arg=<optimized out>) at pthread_create.c:448
ret = <optimized out>
pd = <optimized out>
out = <optimized out>
unwind_buf = {cancel_jmp_buf = {{jmp_buf = {32, 8894544057743421332, -1288, 0, 140726164742416, 140726164742679, -8831356496486092908, -8844535456800460908}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
not_first_call = <optimized out>
#25 0x00007f1710bda924 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
Fixes: 0caed25cd171c611781589b5402161d27d57229c virtio: Call set_features during reset
Signed-off-by: Houqi (Nick) Zuo <hzuo@redhat.com>
---
net/tap-linux.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/net/tap-linux.c b/net/tap-linux.c
index e832810665..24e63a0b54 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -206,15 +206,16 @@ void tap_fd_set_vnet_hdr_len(int fd, int len)
if (ioctl(fd, TUNSETVNETHDRSZ, &len) == -1) {
fprintf(stderr, "TUNSETVNETHDRSZ ioctl() failed: %s. Exiting.\n",
strerror(errno));
- abort();
}
}
int tap_fd_set_vnet_le(int fd, int is_le)
{
int arg = is_le ? 1 : 0;
+ int ret;
- if (!ioctl(fd, TUNSETVNETLE, &arg)) {
+ ret = ioctl(fd, TUNSETVNETLE, &arg);
+ if (!ret) {
return 0;
}
@@ -224,14 +225,16 @@ int tap_fd_set_vnet_le(int fd, int is_le)
}
error_report("TUNSETVNETLE ioctl() failed: %s.", strerror(errno));
- abort();
+ return ret;
}
int tap_fd_set_vnet_be(int fd, int is_be)
{
int arg = is_be ? 1 : 0;
+ int ret;
- if (!ioctl(fd, TUNSETVNETBE, &arg)) {
+ ret = ioctl(fd, TUNSETVNETBE, &arg);
+ if (!ret) {
return 0;
}
@@ -241,7 +244,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)
}
error_report("TUNSETVNETBE ioctl() failed: %s.", strerror(errno));
- abort();
+ return ret;
}
void tap_fd_set_offload(int fd, int csum, int tso4,
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3] net/tap-linux.c: avoid abort when setting invalid fd
2025-09-24 8:50 ` [PATCH v3] " Houqi (Nick) Zuo
@ 2025-09-25 1:01 ` Cindy Lu
0 siblings, 0 replies; 8+ messages in thread
From: Cindy Lu @ 2025-09-25 1:01 UTC (permalink / raw)
To: Houqi (Nick) Zuo; +Cc: qemu-devel, Jason Wang, Michael Tsirkin
On Wed, Sep 24, 2025 at 4:51 PM Houqi (Nick) Zuo <hzuo@redhat.com> wrote:
>
> When QEMU creates a tap device automatically and the tap device is
> manually removed from the host while the guest is running, the tap
> device file descriptor becomes invalid. Later, when the guest executes
> shutdown, the tap_fd_set_vnet_hdr_len() function may be called and
> abort QEMU with a core dump when attempting to use the invalid fd.
>
> This patch removes many abort() calls in this file. If the fd is found
> to be in a bad state (e.g., EBADFD or ENODEV), the related function
> will print an error message.
>
> The expected behavior for this negative test case is that QEMU should
> report an error but continue running rather than aborting.
>
> Testing:
> - Start QEMU with automatically created tap device
> - Manually remove the tap device on the host
> - Execute shutdown in the guest
> - Verify QEMU reports an error but does not abort
>
> (gdb) bt full
> #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
> tid = <optimized out>
> ret = 0
> pd = <optimized out>
> old_mask = {__val = {10}}
> ret = <optimized out>
> #1 0x00007f1710b6bff3 in __pthread_kill_internal (threadid=<optimized out>, signo=6) at pthread_kill.c:78
> #2 0x00007f1710b15f56 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
> ret = <optimized out>
> #3 0x00007f1710afd8fa in __GI_abort () at abort.c:79
> save_stage = 1
> act = {__sigaction_handler = {sa_handler = 0x20, sa_sigaction = 0x20}, sa_mask = {__val = {16929458408262392576, 18446744073709550848, 139737042419943, 139737042419943, 0, 94049703655600, 139737042419943, 139737042670528, 18446744073709550328, 77, 139705603579344, 18446744073709551615, 139737041472378, 139705595179568, 16929458408262392576, 94049679794864}}, sa_flags = 281695456, sa_restorer = 0xa}
> #4 0x000055899a71de58 in tap_fd_set_vnet_hdr_len (fd=<optimized out>, len=10) at ../net/tap-linux.c:204
> #5 tap_set_vnet_hdr_len (nc=<optimized out>, len=10) at ../net/tap.c:269
> s = <optimized out>
> #6 0x000055899a8be67f in qemu_set_vnet_hdr_len (nc=0x2956, len=10588) at ../net/net.c:573
> #7 virtio_net_set_mrg_rx_bufs (n=0x5589a72cfa10, mergeable_rx_bufs=<optimized out>, version_1=<error reading variable: Incompatible types on DWARF stack>, hash_report=<optimized out>) at ../hw/net/virtio-net.c:664
> i = 0
> nc = 0x5589a730ab28
> #8 virtio_net_set_features (vdev=0x5589a72cfa10, features=0) at ../hw/net/virtio-net.c:897
> n = 0x5589a72cfa10
> err = 0x0
> i = 0
> #9 0x000055899a8e4eaa in virtio_set_features_nocheck (vdev=0x5589a72cfa10, val=0) at ../hw/virtio/virtio.c:3079
> k = <optimized out>
> bad = <optimized out>
> #10 virtio_reset (opaque=0x5589a72cfa10) at ../hw/virtio/virtio.c:3184
> vdev = 0x5589a72cfa10
> k = 0x5589a5c162b0
> i = 0
> #11 0x000055899a630d2b in virtio_bus_reset (bus=0x5589a72cf990) at ../hw/virtio/virtio-bus.c:109
> vdev = <optimized out>
> #12 virtio_pci_reset (qdev=0x5589a72c7470) at ../hw/virtio/virtio-pci.c:2311
> proxy = 0x5589a72c7470
> i = 0
> bus = 0x5589a72cf990
> #13 0x000055899a686ded in memory_region_write_accessor (mr=<optimized out>, addr=<optimized out>, value=<optimized out>, size=<optimized out>, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../system/memory.c:490
> tmp = <optimized out>
> #14 0x000055899a686cbc in access_with_adjusted_size (addr=20, value=0x7f0fbedfde00, size=1, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x55899a686d30 <memory_region_write_accessor>, mr=0x5589a72c8040, attrs=...) at ../system/memory.c:566
> print_once_ = false
> access_mask = 255
> access_size = 1
> i = 0
> r = 0
> reentrancy_guard_applied = <optimized out>
> #15 0x000055899a686ac5 in memory_region_dispatch_write (mr=<optimized out>, addr=20, data=<optimized out>, op=<optimized out>, attrs=...) at ../system/memory.c:1545
> size = <optimized out>
> #16 0x000055899a69f7da in flatview_write_continue_step (attrs=..., buf=0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>, len=<optimized out>, mr_addr=20, l=0x7f0fbedfde28, mr=0x5589a72c8040) at ../system/physmem.c:2972
> val = 6
> result = 0
> release_lock = <optimized out>
> #17 0x000055899a697c15 in flatview_write_continue (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., ptr=0x7f1711da6028, len=1, mr_addr=6, l=1, mr=0x0) at ../system/physmem.c:3002
> result = 0
> buf = 0x7f1711da6028 <error: Cannot access memory at address 0x7f1711da6028>
> #18 flatview_write (fv=0x7f0f6c124d90, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3033
> --Type <RET> for more, q to quit, c to continue without paging--
> l = <optimized out>
> mr_addr = 6
> mr = 0x0
> #19 0x000055899a697a91 in address_space_write (as=0x55899bceeba0 <address_space_memory>, addr=61675730370580, attrs=..., buf=0x7f1711da6028, len=1) at ../system/physmem.c:3153
> _rcu_read_auto = 0x1
> result = 0
> fv = 0x2956
> #20 0x000055899a91159b in address_space_rw (addr=10588, attrs=..., buf=0x7f1711da6028, len=0, as=<optimized out>, is_write=<optimized out>) at ../system/physmem.c:3163
> #21 kvm_cpu_exec (cpu=0x5589a5d68b40) at ../accel/kvm/kvm-all.c:3255
> attrs = {secure = 0, space = 0, user = 0, memory = 0, debug = 0, requester_id = 0, pid = 0, address_type = 0, unspecified = false, _reserved1 = 0 '\000', _reserved2 = 0}
> run = 0x7f1711da6000
> ret = <optimized out>
> run_ret = <optimized out>
> #22 0x000055899a9189ca in kvm_vcpu_thread_fn (arg=0x5589a5d68b40) at ../accel/kvm/kvm-accel-ops.c:51
> r = <optimized out>
> cpu = <optimized out>
> #23 0x000055899aba817a in qemu_thread_start (args=0x5589a5d72580) at ../util/qemu-thread-posix.c:393
> __clframe = {__cancel_routine = <optimized out>, __cancel_arg = 0x0, __do_it = 1, __cancel_type = <optimized out>}
> qemu_thread_args = 0x5589a5d72580
> start_routine = 0x55899a918850 <kvm_vcpu_thread_fn>
> arg = 0x5589a5d68b40
> r = 0x0
> #24 0x00007f1710b6a128 in start_thread (arg=<optimized out>) at pthread_create.c:448
> ret = <optimized out>
> pd = <optimized out>
> out = <optimized out>
> unwind_buf = {cancel_jmp_buf = {{jmp_buf = {32, 8894544057743421332, -1288, 0, 140726164742416, 140726164742679, -8831356496486092908, -8844535456800460908}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
> not_first_call = <optimized out>
> #25 0x00007f1710bda924 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
>
> Fixes: 0caed25cd171c611781589b5402161d27d57229c virtio: Call set_features during reset
>
i think the format of fix should be
“Fixes: <at-least-12-digits-of-SHA-commit-id> (“Fixed commit subject”)”
> Signed-off-by: Houqi (Nick) Zuo <hzuo@redhat.com>
> ---
> net/tap-linux.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index e832810665..24e63a0b54 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -206,15 +206,16 @@ void tap_fd_set_vnet_hdr_len(int fd, int len)
> if (ioctl(fd, TUNSETVNETHDRSZ, &len) == -1) {
> fprintf(stderr, "TUNSETVNETHDRSZ ioctl() failed: %s. Exiting.\n",
> strerror(errno));
> - abort();
> }
> }
>
> int tap_fd_set_vnet_le(int fd, int is_le)
> {
> int arg = is_le ? 1 : 0;
> + int ret;
>
> - if (!ioctl(fd, TUNSETVNETLE, &arg)) {
> + ret = ioctl(fd, TUNSETVNETLE, &arg);
> + if (!ret) {
> return 0;
> }
>
> @@ -224,14 +225,16 @@ int tap_fd_set_vnet_le(int fd, int is_le)
> }
>
> error_report("TUNSETVNETLE ioctl() failed: %s.", strerror(errno));
> - abort();
> + return ret;
> }
>
> int tap_fd_set_vnet_be(int fd, int is_be)
> {
> int arg = is_be ? 1 : 0;
> + int ret;
>
> - if (!ioctl(fd, TUNSETVNETBE, &arg)) {
> + ret = ioctl(fd, TUNSETVNETBE, &arg);
> + if (!ret) {
> return 0;
> }
>
> @@ -241,7 +244,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)
> }
>
> error_report("TUNSETVNETBE ioctl() failed: %s.", strerror(errno));
> - abort();
> + return ret;
> }
>
> void tap_fd_set_offload(int fd, int csum, int tso4,
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread