Re: Bug with Windows tap network when running qemu-system-ppc with Mac OS 9 guest

[Jason Wang <jasowang@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4155 bytes --]


在 2021/3/26 下午4:21, BALATON Zoltan 写道:
> On Fri, 26 Mar 2021, Howard Spoelstra wrote:
>> On Fri, Mar 26, 2021 at 2:50 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>>>
>>> Hi Howard,
>>>
>>> On Fri, Mar 26, 2021 at 1:35 AM Peter Maydell 
>>> <peter.maydell@linaro.org> wrote:
>>>>
>>>> (adding the relevant people to the cc list)
>>>>
>>>> On Thu, 25 Mar 2021 at 17:26, Howard Spoelstra <hsp.cat7@gmail.com> 
>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> When running qemu-system-ppc with Mac OS guest, the guest crashes 
>>>>> when
>>>>> using a tap network connection. Openvpn 2.4.9-I601-win10 is installed
>>>>> with TAP-Windows 9.24.2. A tap connection called TapQemu is bridged
>>>>> with the default ethernet connection. It gets activated when I start
>>>>> qemu.
>>>>>
>>>>> To reproduce, compile qemu-system-ppc from current source and run:
>>>>>
>>>>> qemu-system-ppc.exe ^
>>>>> -L pc-bios ^
>>>>> -M mac99 ^
>>>>> -m 128 ^
>>>>> -sdl -serial stdio ^
>>>>> -boot c ^
>>>>> -drive file=C:\Mac-disks\9.2.img,format=raw,media=disk ^
>>>>> -device sungem,netdev=network01 -netdev 
>>>>> tap,ifname=TapQemu,id=network01
>>>>>
>>>>> I bisected to the commit below. Thanks for looking into this.
>>>
>>> Thanks for reporting.
>>>
>>> Can you please provide some further information:
>>>
>>> 1. Does "-net user" work on Windows?
>>> 2. If running QEMU under Linux, does "-net tap" or "-net user" work?
>>>
>>> Regards,
>>> Bin
>>
>> Hello Bin,
>>
>> Thanks for getting back to me. I forgot to mention that reverting the
>> above patch restores functionality. And that other applications using
>> the same tap device work correctly.
>> In answer to your questions:
>>
>> 1. Yes, slirp works on Windows 10 with this setup.
>> 2. Yes, in Linux both tap and slirp work.
>>
>> My Windows build is done with a fully up to date msys2 installation.
>>
>> I tried to debug in Windows:
>> (gdb) run
>> Starting program: c:\qemu-master-msys2\qemu-system-ppc.exe -L pc-bios
>> -M mac99 -m 128 -sdl -serial stdio -boot c -drive
>> "file=C:\Mac-disks\9.2-usb-pci-ddk.img,format=raw,media=disk" -device
>> "sungem,netdev=network01" -netdev "tap,ifname=TapQemu,id=network01" -S
>> [New Thread 13304.0x1f00]
>> [New Thread 13304.0x2f84]
>> [New Thread 13304.0x3524]
>> [New Thread 13304.0x2b8c]
>> [New Thread 13304.0x368c]
>> [New Thread 13304.0x3668]
>> [New Thread 13304.0xf4c]
>> [New Thread 13304.0x49c]
>> [New Thread 13304.0x1d4c]
>> [New Thread 13304.0x7fc]
>> [Thread 13304.0x7fc exited with code 0]
>> [New Thread 13304.0x357c]
>> [New Thread 13304.0x7c0]
>> [New Thread 13304.0x3564]
>> [New Thread 13304.0x26f4]
>> [New Thread 13304.0x2f68]
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x00007ffb9edea991 in ?? () from c:\qemu-master-msys2\libglib-2.0-0.dll
>> (gdb) bt
>> #0  0x00007ffb9edea991 in ?? () from 
>> c:\qemu-master-msys2\libglib-2.0-0.dll
>> #1  0x000800000480bf50 in ?? ()
>> Backtrace stopped: previous frame inner to this frame (corrupt stack?)
>> (gdb)
>>
>> Even before I could attach to the process.
>
> If you run QEMU under gdb you don't have to attach to it but to get a 
> meaningful backtrace you should configure and compile QEMU with 
> --enable-debug (this will make it run slower so not recommended 
> normally but for debugging that would be needed). If the stack is 
> really corrupted then you may not get a useful backtrace or it may be 
> a problem with gdb on Windows. I've found that gdb on Windows works 
> for simple things but could give bad results for more complex stuff. 
> WinDbg may be better but it's harder to use (needs some registry 
> change I think to enable core dumps then you could open and analyze 
> core dumps with it or it should be able to run command directly but I 
> don't know how that works).
>
> Another idea: maybe you could check other threads in gdb. Not sure if 
> that would reveal anything but may worth a try. I think the commands 
> you need are "info threads" and "apply all bt" or something similar.
>
> Regards,
> BALATON Zoltan
>

It looks to me the patch tires to recycle a temporary buffer to tap thread.

Please try to attached fix to see it if works.

Thanks


[-- Attachment #2: 0001-tap-win32-correctly-recycle-buffers.patch --]
[-- Type: text/plain, Size: 1658 bytes --]

From bdd7b4b7e13264f30d4abbc6f0f32c8af935ff17 Mon Sep 17 00:00:00 2001
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 26 Mar 2021 16:46:43 +0800
Subject: [PATCH] tap-win32: correctly recycle buffers

Commit 969e50b61a28 ("net: Pad short frames to minimum size before
sending from SLiRP/TAP") tries to pad frames but try to recyle the
local array that is used for padding to tap thread. This patch fixes
this by recyling the original buffer.

Fixes: 969e50b61a28 ("net: Pad short frames to minimum size before sending from SLiRP/TAP")
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/tap-win32.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/tap-win32.c b/net/tap-win32.c
index d7c2a8759c..95dacbd171 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -686,7 +686,7 @@ static ssize_t tap_receive(NetClientState *nc, const uint8_t *buf, size_t size)
 static void tap_win32_send(void *opaque)
 {
     TAPState *s = opaque;
-    uint8_t *buf;
+    uint8_t *buf, orig_buf;
     int max_size = 4096;
     int size;
     uint8_t min_pkt[ETH_ZLEN];
@@ -694,6 +694,8 @@ static void tap_win32_send(void *opaque)
 
     size = tap_win32_read(s->handle, &buf, max_size);
     if (size > 0) {
+        orig_buf = buf;
+
         if (!s->nc.peer->do_not_pad) {
             if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
                 buf = min_pkt;
@@ -702,7 +704,7 @@ static void tap_win32_send(void *opaque)
         }
 
         qemu_send_packet(&s->nc, buf, size);
-        tap_win32_free_buffer(s->handle, buf);
+        tap_win32_free_buffer(s->handle, orig_buf);
     }
 }
 
-- 
2.24.3 (Apple Git-128)