All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: netdev@vger.kernel.org, jpmenil@gmail.com,
	John Fastabend <john.fastabend@gmail.com>,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org
Subject: Re: [PATCH net] virtio-net: serialize tx routine during reset
Date: Wed, 28 Jun 2017 05:00:24 +0300	[thread overview]
Message-ID: <20170628045956-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <1498614663-7711-1-git-send-email-jasowang@redhat.com>

On Wed, Jun 28, 2017 at 09:51:03AM +0800, Jason Wang wrote:
> We don't hold any tx lock when trying to disable TX during reset, this
> would lead a use after free since ndo_start_xmit() tries to access
> the virtqueue which has already been freed. Fix this by using
> netif_tx_disable() before freeing the vqs, this could make sure no tx
> after vq freeing.
> 
> Reported-by: Jean-Philippe Menil <jpmenil@gmail.com>
> Tested-by: Jean-Philippe Menil <jpmenil@gmail.com>
> Fixes commit f600b6905015 ("virtio_net: Add XDP support")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Thanks a lot Jason. I think this is needed in stable as well.

> ---
>  drivers/net/virtio_net.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index a871f45..143d8a9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1797,6 +1797,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
>  	flush_work(&vi->config_work);
>  
>  	netif_device_detach(vi->dev);
> +	netif_tx_disable(vi->dev);
>  	cancel_delayed_work_sync(&vi->refill);
>  
>  	if (netif_running(vi->dev)) {
> -- 
> 2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: virtualization@lists.linux-foundation.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	jpmenil@gmail.com, John Fastabend <john.fastabend@gmail.com>
Subject: Re: [PATCH net] virtio-net: serialize tx routine during reset
Date: Wed, 28 Jun 2017 05:00:24 +0300	[thread overview]
Message-ID: <20170628045956-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <1498614663-7711-1-git-send-email-jasowang@redhat.com>

On Wed, Jun 28, 2017 at 09:51:03AM +0800, Jason Wang wrote:
> We don't hold any tx lock when trying to disable TX during reset, this
> would lead a use after free since ndo_start_xmit() tries to access
> the virtqueue which has already been freed. Fix this by using
> netif_tx_disable() before freeing the vqs, this could make sure no tx
> after vq freeing.
> 
> Reported-by: Jean-Philippe Menil <jpmenil@gmail.com>
> Tested-by: Jean-Philippe Menil <jpmenil@gmail.com>
> Fixes commit f600b6905015 ("virtio_net: Add XDP support")
> Cc: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Thanks a lot Jason. I think this is needed in stable as well.

> ---
>  drivers/net/virtio_net.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index a871f45..143d8a9 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1797,6 +1797,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
>  	flush_work(&vi->config_work);
>  
>  	netif_device_detach(vi->dev);
> +	netif_tx_disable(vi->dev);
>  	cancel_delayed_work_sync(&vi->refill);
>  
>  	if (netif_running(vi->dev)) {
> -- 
> 2.7.4

  reply	other threads:[~2017-06-28  2:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-28  1:51 [PATCH net] virtio-net: serialize tx routine during reset Jason Wang
2017-06-28  1:51 ` Jason Wang
2017-06-28  2:00 ` Michael S. Tsirkin [this message]
2017-06-28  2:00   ` Michael S. Tsirkin
2017-06-28  3:02 ` [net] " McCabe, Robert J
2017-06-29 16:52 ` [PATCH net] " David Miller
2017-06-29 16:52   ` David Miller

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=20170628045956-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=john.fastabend@gmail.com \
    --cc=jpmenil@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.