Re: [PATCH] net/net.c: add tap device fd validity check to prevent abort on deleted device

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|