qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jason Wang <jasowang@redhat.com>
To: Cole Robinson <crobinso@redhat.com>, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [PATCH-for-6.0] net: tap: fix crash on hotplug
Date: Thu, 22 Apr 2021 10:25:26 +0800	[thread overview]
Message-ID: <32dd3af8-365c-91ba-6a1e-d24cb60e9e35@redhat.com> (raw)
In-Reply-To: <3f6be9c84782a0943ea21a8a6f8a5d055b65f2d5.1619018363.git.crobinso@redhat.com>


在 2021/4/21 下午11:22, Cole Robinson 写道:
> Attempting to hotplug a tap nic with libvirt will crash qemu:
>
> $ sudo virsh attach-interface f32 network default
> error: Failed to attach interface
> error: Unable to read from monitor: Connection reset by peer
>
> 0x000055875b7f3a99 in tap_send (opaque=0x55875e39eae0) at ../net/tap.c:206
> 206	        if (!s->nc.peer->do_not_pad) {
> gdb$ bt
>
> s->nc.peer may not be set at this point. This seems to be an
> expected case, as qemu_send_packet_* explicitly checks for NULL
> s->nc.peer later.
>
> Fix it by checking for s->nc.peer here too. Padding is applied if
> s->nc.peer is not set.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1949786
> Fixes: 969e50b61a2
>
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
> * Or should we skip padding if nc.peer is unset?


I think so, the padding is for the peer.


> I didn't dig into it
> * tap-win3.c and slirp.c may need a similar fix, but the slirp case
>    didn't crash in a simple test.


Yes, the reason is because there's no packet go through slirp I think.

Thanks.


>
>   net/tap.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/tap.c b/net/tap.c
> index dd42ac6134..937559dbb8 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -203,7 +203,7 @@ static void tap_send(void *opaque)
>               size -= s->host_vnet_hdr_len;
>           }
>   
> -        if (!s->nc.peer->do_not_pad) {
> +        if (!s->nc.peer || !s->nc.peer->do_not_pad) {
>               if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
>                   buf = min_pkt;
>                   size = min_pktsz;



      parent reply	other threads:[~2021-04-22  2:26 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21 15:22 [PATCH-for-6.0] net: tap: fix crash on hotplug Cole Robinson
2021-04-21 16:36 ` Philippe Mathieu-Daudé
2021-04-22  4:29   ` Bin Meng
2021-04-22  9:36     ` Peter Maydell
2021-04-22  9:42       ` Bin Meng
2021-04-22 21:34         ` Cole Robinson
2021-04-23  1:43           ` Jason Wang
2021-04-21 19:54 ` Peter Maydell
2021-04-21 22:27   ` Cole Robinson
2021-04-22  2:25 ` Jason Wang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=32dd3af8-365c-91ba-6a1e-d24cb60e9e35@redhat.com \
    --to=jasowang@redhat.com \
    --cc=crobinso@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).