All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] vhost_net: fix OoB on sendmsg() failure.
@ 2021-09-08 11:42 ` Paolo Abeni
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Abeni @ 2021-09-08 11:42 UTC (permalink / raw)
  To: netdev; +Cc: Michael S. Tsirkin, Jason Wang, kvm, virtualization

If the sendmsg() call in vhost_tx_batch() fails, both the 'batched_xdp'
and 'done_idx' indexes are left unchanged. If such failure happens
when batched_xdp == VHOST_NET_BATCH, the next call to
vhost_net_build_xdp() will access and write memory outside the xdp
buffers area.

Since sendmsg() can only error with EBADFD, this change addresses the
issue explicitly freeing the XDP buffers batch on error.

Fixes: 0a0be13b8fe2 ("vhost_net: batch submitting XDP buffers to underlayer sockets")
Suggested-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Note: my understanding is that this should go through MST's tree, please
educate me otherwise, thanks!
---
 drivers/vhost/net.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 3a249ee7e144..28ef323882fb 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -467,7 +467,7 @@ static void vhost_tx_batch(struct vhost_net *net,
 		.num = nvq->batched_xdp,
 		.ptr = nvq->xdp,
 	};
-	int err;
+	int i, err;
 
 	if (nvq->batched_xdp == 0)
 		goto signal_used;
@@ -476,6 +476,15 @@ static void vhost_tx_batch(struct vhost_net *net,
 	err = sock->ops->sendmsg(sock, msghdr, 0);
 	if (unlikely(err < 0)) {
 		vq_err(&nvq->vq, "Fail to batch sending packets\n");
+
+		/* free pages owned by XDP; since this is an unlikely error path,
+		 * keep it simple and avoid more complex bulk update for the
+		 * used pages
+		 */
+		for (i = 0; i < nvq->batched_xdp; ++i)
+			put_page(virt_to_head_page(nvq->xdp[i].data));
+		nvq->batched_xdp = 0;
+		nvq->done_idx = 0;
 		return;
 	}
 
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH] vhost_net: fix OoB on sendmsg() failure.
@ 2021-09-08 11:42 ` Paolo Abeni
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Abeni @ 2021-09-08 11:42 UTC (permalink / raw)
  To: netdev; +Cc: virtualization, kvm, Michael S. Tsirkin

If the sendmsg() call in vhost_tx_batch() fails, both the 'batched_xdp'
and 'done_idx' indexes are left unchanged. If such failure happens
when batched_xdp == VHOST_NET_BATCH, the next call to
vhost_net_build_xdp() will access and write memory outside the xdp
buffers area.

Since sendmsg() can only error with EBADFD, this change addresses the
issue explicitly freeing the XDP buffers batch on error.

Fixes: 0a0be13b8fe2 ("vhost_net: batch submitting XDP buffers to underlayer sockets")
Suggested-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Note: my understanding is that this should go through MST's tree, please
educate me otherwise, thanks!
---
 drivers/vhost/net.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 3a249ee7e144..28ef323882fb 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -467,7 +467,7 @@ static void vhost_tx_batch(struct vhost_net *net,
 		.num = nvq->batched_xdp,
 		.ptr = nvq->xdp,
 	};
-	int err;
+	int i, err;
 
 	if (nvq->batched_xdp == 0)
 		goto signal_used;
@@ -476,6 +476,15 @@ static void vhost_tx_batch(struct vhost_net *net,
 	err = sock->ops->sendmsg(sock, msghdr, 0);
 	if (unlikely(err < 0)) {
 		vq_err(&nvq->vq, "Fail to batch sending packets\n");
+
+		/* free pages owned by XDP; since this is an unlikely error path,
+		 * keep it simple and avoid more complex bulk update for the
+		 * used pages
+		 */
+		for (i = 0; i < nvq->batched_xdp; ++i)
+			put_page(virt_to_head_page(nvq->xdp[i].data));
+		nvq->batched_xdp = 0;
+		nvq->done_idx = 0;
 		return;
 	}
 
-- 
2.26.3

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] vhost_net: fix OoB on sendmsg() failure.
  2021-09-08 11:42 ` Paolo Abeni
@ 2021-09-09  2:52   ` Jason Wang
  -1 siblings, 0 replies; 5+ messages in thread
From: Jason Wang @ 2021-09-09  2:52 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, Michael S. Tsirkin, kvm, virtualization

On Wed, Sep 8, 2021 at 7:42 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> If the sendmsg() call in vhost_tx_batch() fails, both the 'batched_xdp'
> and 'done_idx' indexes are left unchanged. If such failure happens
> when batched_xdp == VHOST_NET_BATCH, the next call to
> vhost_net_build_xdp() will access and write memory outside the xdp
> buffers area.
>
> Since sendmsg() can only error with EBADFD, this change addresses the
> issue explicitly freeing the XDP buffers batch on error.
>
> Fixes: 0a0be13b8fe2 ("vhost_net: batch submitting XDP buffers to underlayer sockets")
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> Note: my understanding is that this should go through MST's tree, please
> educate me otherwise, thanks!
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks!

>  drivers/vhost/net.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 3a249ee7e144..28ef323882fb 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -467,7 +467,7 @@ static void vhost_tx_batch(struct vhost_net *net,
>                 .num = nvq->batched_xdp,
>                 .ptr = nvq->xdp,
>         };
> -       int err;
> +       int i, err;
>
>         if (nvq->batched_xdp == 0)
>                 goto signal_used;
> @@ -476,6 +476,15 @@ static void vhost_tx_batch(struct vhost_net *net,
>         err = sock->ops->sendmsg(sock, msghdr, 0);
>         if (unlikely(err < 0)) {
>                 vq_err(&nvq->vq, "Fail to batch sending packets\n");
> +
> +               /* free pages owned by XDP; since this is an unlikely error path,
> +                * keep it simple and avoid more complex bulk update for the
> +                * used pages
> +                */
> +               for (i = 0; i < nvq->batched_xdp; ++i)
> +                       put_page(virt_to_head_page(nvq->xdp[i].data));
> +               nvq->batched_xdp = 0;
> +               nvq->done_idx = 0;
>                 return;
>         }
>
> --
> 2.26.3
>


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] vhost_net: fix OoB on sendmsg() failure.
@ 2021-09-09  2:52   ` Jason Wang
  0 siblings, 0 replies; 5+ messages in thread
From: Jason Wang @ 2021-09-09  2:52 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, virtualization, kvm, Michael S. Tsirkin

On Wed, Sep 8, 2021 at 7:42 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> If the sendmsg() call in vhost_tx_batch() fails, both the 'batched_xdp'
> and 'done_idx' indexes are left unchanged. If such failure happens
> when batched_xdp == VHOST_NET_BATCH, the next call to
> vhost_net_build_xdp() will access and write memory outside the xdp
> buffers area.
>
> Since sendmsg() can only error with EBADFD, this change addresses the
> issue explicitly freeing the XDP buffers batch on error.
>
> Fixes: 0a0be13b8fe2 ("vhost_net: batch submitting XDP buffers to underlayer sockets")
> Suggested-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> Note: my understanding is that this should go through MST's tree, please
> educate me otherwise, thanks!
> ---

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks!

>  drivers/vhost/net.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 3a249ee7e144..28ef323882fb 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -467,7 +467,7 @@ static void vhost_tx_batch(struct vhost_net *net,
>                 .num = nvq->batched_xdp,
>                 .ptr = nvq->xdp,
>         };
> -       int err;
> +       int i, err;
>
>         if (nvq->batched_xdp == 0)
>                 goto signal_used;
> @@ -476,6 +476,15 @@ static void vhost_tx_batch(struct vhost_net *net,
>         err = sock->ops->sendmsg(sock, msghdr, 0);
>         if (unlikely(err < 0)) {
>                 vq_err(&nvq->vq, "Fail to batch sending packets\n");
> +
> +               /* free pages owned by XDP; since this is an unlikely error path,
> +                * keep it simple and avoid more complex bulk update for the
> +                * used pages
> +                */
> +               for (i = 0; i < nvq->batched_xdp; ++i)
> +                       put_page(virt_to_head_page(nvq->xdp[i].data));
> +               nvq->batched_xdp = 0;
> +               nvq->done_idx = 0;
>                 return;
>         }
>
> --
> 2.26.3
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] vhost_net: fix OoB on sendmsg() failure.
  2021-09-08 11:42 ` Paolo Abeni
  (?)
  (?)
@ 2021-09-09 10:00 ` patchwork-bot+netdevbpf
  -1 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-09-09 10:00 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, mst, jasowang, kvm, virtualization

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Wed,  8 Sep 2021 13:42:09 +0200 you wrote:
> If the sendmsg() call in vhost_tx_batch() fails, both the 'batched_xdp'
> and 'done_idx' indexes are left unchanged. If such failure happens
> when batched_xdp == VHOST_NET_BATCH, the next call to
> vhost_net_build_xdp() will access and write memory outside the xdp
> buffers area.
> 
> Since sendmsg() can only error with EBADFD, this change addresses the
> issue explicitly freeing the XDP buffers batch on error.
> 
> [...]

Here is the summary with links:
  - vhost_net: fix OoB on sendmsg() failure.
    https://git.kernel.org/netdev/net/c/3c4cea8fa7f7

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-09 10:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 11:42 [PATCH] vhost_net: fix OoB on sendmsg() failure Paolo Abeni
2021-09-08 11:42 ` Paolo Abeni
2021-09-09  2:52 ` Jason Wang
2021-09-09  2:52   ` Jason Wang
2021-09-09 10:00 ` patchwork-bot+netdevbpf

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.