* [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value
2022-04-20 20:07 [PATCH 0/5] hv_sock: Hardening changes Andrea Parri (Microsoft)
@ 2022-04-20 20:07 ` Andrea Parri (Microsoft)
2022-04-20 23:07 ` Michael Kelley (LINUX) via Virtualization
2022-04-21 13:50 ` Stefano Garzarella
2022-04-20 20:07 ` [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer Andrea Parri (Microsoft)
` (3 subsequent siblings)
4 siblings, 2 replies; 26+ messages in thread
From: Andrea Parri (Microsoft) @ 2022-04-20 20:07 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, Stefano Garzarella, David Miller,
Jakub Kicinski, Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel,
Andrea Parri (Microsoft)
The function returns NULL if the ring buffer doesn't contain enough
readable bytes to constitute a packet descriptor. The ring buffer's
write_index is in memory which is shared with the Hyper-V host, an
erroneous or malicious host could thus change its value and overturn
the result of hvs_stream_has_data().
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
net/vmw_vsock/hyperv_transport.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index e111e13b66604..943352530936e 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -603,6 +603,8 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
if (need_refill) {
hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
+ if (!hvs->recv_desc)
+ return -ENOBUFS;
ret = hvs_update_recv_data(hvs);
if (ret)
return ret;
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* RE: [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value
2022-04-20 20:07 ` [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value Andrea Parri (Microsoft)
@ 2022-04-20 23:07 ` Michael Kelley (LINUX) via Virtualization
2022-04-21 13:50 ` Stefano Garzarella
1 sibling, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) @ 2022-04-20 23:07 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> The function returns NULL if the ring buffer doesn't contain enough
> readable bytes to constitute a packet descriptor. The ring buffer's
> write_index is in memory which is shared with the Hyper-V host, an
> erroneous or malicious host could thus change its value and overturn
> the result of hvs_stream_has_data().
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> net/vmw_vsock/hyperv_transport.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index e111e13b66604..943352530936e 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -603,6 +603,8 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
>
> if (need_refill) {
> hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
> + if (!hvs->recv_desc)
> + return -ENOBUFS;
> ret = hvs_update_recv_data(hvs);
> if (ret)
> return ret;
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 26+ messages in thread* RE: [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value
@ 2022-04-20 23:07 ` Michael Kelley (LINUX) via Virtualization
0 siblings, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) via Virtualization @ 2022-04-20 23:07 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-hyperv, linux-kernel, virtualization
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> The function returns NULL if the ring buffer doesn't contain enough
> readable bytes to constitute a packet descriptor. The ring buffer's
> write_index is in memory which is shared with the Hyper-V host, an
> erroneous or malicious host could thus change its value and overturn
> the result of hvs_stream_has_data().
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> net/vmw_vsock/hyperv_transport.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index e111e13b66604..943352530936e 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -603,6 +603,8 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
>
> if (need_refill) {
> hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
> + if (!hvs->recv_desc)
> + return -ENOBUFS;
> ret = hvs_update_recv_data(hvs);
> if (ret)
> return ret;
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value
2022-04-20 20:07 ` [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value Andrea Parri (Microsoft)
@ 2022-04-21 13:50 ` Stefano Garzarella
2022-04-21 13:50 ` Stefano Garzarella
1 sibling, 0 replies; 26+ messages in thread
From: Stefano Garzarella @ 2022-04-21 13:50 UTC (permalink / raw)
To: Andrea Parri (Microsoft)
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, David Miller, Jakub Kicinski,
Paolo Abeni, linux-hyperv, virtualization, netdev, linux-kernel
On Wed, Apr 20, 2022 at 10:07:16PM +0200, Andrea Parri (Microsoft) wrote:
>The function returns NULL if the ring buffer doesn't contain enough
>readable bytes to constitute a packet descriptor. The ring buffer's
>write_index is in memory which is shared with the Hyper-V host, an
>erroneous or malicious host could thus change its value and overturn
>the result of hvs_stream_has_data().
>
>Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
>---
> net/vmw_vsock/hyperv_transport.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index e111e13b66604..943352530936e 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -603,6 +603,8 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
>
> if (need_refill) {
> hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
>+ if (!hvs->recv_desc)
>+ return -ENOBUFS;
> ret = hvs_update_recv_data(hvs);
> if (ret)
> return ret;
>--
>2.25.1
>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value
@ 2022-04-21 13:50 ` Stefano Garzarella
0 siblings, 0 replies; 26+ messages in thread
From: Stefano Garzarella @ 2022-04-21 13:50 UTC (permalink / raw)
To: Andrea Parri (Microsoft)
Cc: Wei Liu, Paolo Abeni, Stephen Hemminger, netdev, Haiyang Zhang,
Dexuan Cui, linux-hyperv, Michael Kelley, Jakub Kicinski,
virtualization, David Miller, linux-kernel
On Wed, Apr 20, 2022 at 10:07:16PM +0200, Andrea Parri (Microsoft) wrote:
>The function returns NULL if the ring buffer doesn't contain enough
>readable bytes to constitute a packet descriptor. The ring buffer's
>write_index is in memory which is shared with the Hyper-V host, an
>erroneous or malicious host could thus change its value and overturn
>the result of hvs_stream_has_data().
>
>Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
>---
> net/vmw_vsock/hyperv_transport.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index e111e13b66604..943352530936e 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -603,6 +603,8 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
>
> if (need_refill) {
> hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
>+ if (!hvs->recv_desc)
>+ return -ENOBUFS;
> ret = hvs_update_recv_data(hvs);
> if (ret)
> return ret;
>--
>2.25.1
>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer
2022-04-20 20:07 [PATCH 0/5] hv_sock: Hardening changes Andrea Parri (Microsoft)
2022-04-20 20:07 ` [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value Andrea Parri (Microsoft)
@ 2022-04-20 20:07 ` Andrea Parri (Microsoft)
2022-04-20 23:08 ` Michael Kelley (LINUX) via Virtualization
2022-04-21 13:58 ` Stefano Garzarella
2022-04-20 20:07 ` [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values Andrea Parri (Microsoft)
` (2 subsequent siblings)
4 siblings, 2 replies; 26+ messages in thread
From: Andrea Parri (Microsoft) @ 2022-04-20 20:07 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, Stefano Garzarella, David Miller,
Jakub Kicinski, Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel,
Andrea Parri (Microsoft)
Pointers to VMbus packets sent by Hyper-V are used by the hv_sock driver
within the guest VM. Hyper-V can send packets with erroneous values or
modify packet fields after they are processed by the guest. To defend
against these scenarios, copy the incoming packet after validating its
length and offset fields using hv_pkt_iter_{first,next}(). In this way,
the packet can no longer be modified by the host.
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
net/vmw_vsock/hyperv_transport.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 943352530936e..8c37d07017fc4 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -78,6 +78,9 @@ struct hvs_send_buf {
ALIGN((payload_len), 8) + \
VMBUS_PKT_TRAILER_SIZE)
+/* Upper bound on the size of a VMbus packet for hv_sock */
+#define HVS_MAX_PKT_SIZE HVS_PKT_LEN(HVS_MTU_SIZE)
+
union hvs_service_id {
guid_t srv_id;
@@ -378,6 +381,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
}
+ chan->max_pkt_size = HVS_MAX_PKT_SIZE;
+
ret = vmbus_open(chan, sndbuf, rcvbuf, NULL, 0, hvs_channel_cb,
conn_from_host ? new : sk);
if (ret != 0) {
@@ -602,7 +607,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
return -EOPNOTSUPP;
if (need_refill) {
- hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
+ hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
if (!hvs->recv_desc)
return -ENOBUFS;
ret = hvs_update_recv_data(hvs);
@@ -618,7 +623,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
hvs->recv_data_len -= to_read;
if (hvs->recv_data_len == 0) {
- hvs->recv_desc = hv_pkt_iter_next_raw(hvs->chan, hvs->recv_desc);
+ hvs->recv_desc = hv_pkt_iter_next(hvs->chan, hvs->recv_desc);
if (hvs->recv_desc) {
ret = hvs_update_recv_data(hvs);
if (ret)
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* RE: [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer
2022-04-20 20:07 ` [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer Andrea Parri (Microsoft)
@ 2022-04-20 23:08 ` Michael Kelley (LINUX) via Virtualization
2022-04-21 13:58 ` Stefano Garzarella
1 sibling, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) @ 2022-04-20 23:08 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> Pointers to VMbus packets sent by Hyper-V are used by the hv_sock driver
> within the guest VM. Hyper-V can send packets with erroneous values or
> modify packet fields after they are processed by the guest. To defend
> against these scenarios, copy the incoming packet after validating its
> length and offset fields using hv_pkt_iter_{first,next}(). In this way,
> the packet can no longer be modified by the host.
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> net/vmw_vsock/hyperv_transport.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index 943352530936e..8c37d07017fc4 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -78,6 +78,9 @@ struct hvs_send_buf {
> ALIGN((payload_len), 8) + \
> VMBUS_PKT_TRAILER_SIZE)
>
> +/* Upper bound on the size of a VMbus packet for hv_sock */
> +#define HVS_MAX_PKT_SIZE HVS_PKT_LEN(HVS_MTU_SIZE)
> +
> union hvs_service_id {
> guid_t srv_id;
>
> @@ -378,6 +381,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
> }
>
> + chan->max_pkt_size = HVS_MAX_PKT_SIZE;
> +
> ret = vmbus_open(chan, sndbuf, rcvbuf, NULL, 0, hvs_channel_cb,
> conn_from_host ? new : sk);
> if (ret != 0) {
> @@ -602,7 +607,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
> return -EOPNOTSUPP;
>
> if (need_refill) {
> - hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
> + hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
> if (!hvs->recv_desc)
> return -ENOBUFS;
> ret = hvs_update_recv_data(hvs);
> @@ -618,7 +623,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
>
> hvs->recv_data_len -= to_read;
> if (hvs->recv_data_len == 0) {
> - hvs->recv_desc = hv_pkt_iter_next_raw(hvs->chan, hvs->recv_desc);
> + hvs->recv_desc = hv_pkt_iter_next(hvs->chan, hvs->recv_desc);
> if (hvs->recv_desc) {
> ret = hvs_update_recv_data(hvs);
> if (ret)
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 26+ messages in thread* RE: [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer
@ 2022-04-20 23:08 ` Michael Kelley (LINUX) via Virtualization
0 siblings, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) via Virtualization @ 2022-04-20 23:08 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-hyperv, linux-kernel, virtualization
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> Pointers to VMbus packets sent by Hyper-V are used by the hv_sock driver
> within the guest VM. Hyper-V can send packets with erroneous values or
> modify packet fields after they are processed by the guest. To defend
> against these scenarios, copy the incoming packet after validating its
> length and offset fields using hv_pkt_iter_{first,next}(). In this way,
> the packet can no longer be modified by the host.
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> net/vmw_vsock/hyperv_transport.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index 943352530936e..8c37d07017fc4 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -78,6 +78,9 @@ struct hvs_send_buf {
> ALIGN((payload_len), 8) + \
> VMBUS_PKT_TRAILER_SIZE)
>
> +/* Upper bound on the size of a VMbus packet for hv_sock */
> +#define HVS_MAX_PKT_SIZE HVS_PKT_LEN(HVS_MTU_SIZE)
> +
> union hvs_service_id {
> guid_t srv_id;
>
> @@ -378,6 +381,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
> }
>
> + chan->max_pkt_size = HVS_MAX_PKT_SIZE;
> +
> ret = vmbus_open(chan, sndbuf, rcvbuf, NULL, 0, hvs_channel_cb,
> conn_from_host ? new : sk);
> if (ret != 0) {
> @@ -602,7 +607,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
> return -EOPNOTSUPP;
>
> if (need_refill) {
> - hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
> + hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
> if (!hvs->recv_desc)
> return -ENOBUFS;
> ret = hvs_update_recv_data(hvs);
> @@ -618,7 +623,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
>
> hvs->recv_data_len -= to_read;
> if (hvs->recv_data_len == 0) {
> - hvs->recv_desc = hv_pkt_iter_next_raw(hvs->chan, hvs->recv_desc);
> + hvs->recv_desc = hv_pkt_iter_next(hvs->chan, hvs->recv_desc);
> if (hvs->recv_desc) {
> ret = hvs_update_recv_data(hvs);
> if (ret)
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer
2022-04-20 20:07 ` [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer Andrea Parri (Microsoft)
@ 2022-04-21 13:58 ` Stefano Garzarella
2022-04-21 13:58 ` Stefano Garzarella
1 sibling, 0 replies; 26+ messages in thread
From: Stefano Garzarella @ 2022-04-21 13:58 UTC (permalink / raw)
To: Andrea Parri (Microsoft)
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, David Miller, Jakub Kicinski,
Paolo Abeni, linux-hyperv, virtualization, netdev, linux-kernel
On Wed, Apr 20, 2022 at 10:07:17PM +0200, Andrea Parri (Microsoft) wrote:
>Pointers to VMbus packets sent by Hyper-V are used by the hv_sock driver
>within the guest VM. Hyper-V can send packets with erroneous values or
>modify packet fields after they are processed by the guest. To defend
>against these scenarios, copy the incoming packet after validating its
>length and offset fields using hv_pkt_iter_{first,next}(). In this way,
>the packet can no longer be modified by the host.
>
>Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
>---
> net/vmw_vsock/hyperv_transport.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 943352530936e..8c37d07017fc4 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -78,6 +78,9 @@ struct hvs_send_buf {
> ALIGN((payload_len), 8) + \
> VMBUS_PKT_TRAILER_SIZE)
>
>+/* Upper bound on the size of a VMbus packet for hv_sock */
>+#define HVS_MAX_PKT_SIZE HVS_PKT_LEN(HVS_MTU_SIZE)
>+
> union hvs_service_id {
> guid_t srv_id;
>
>@@ -378,6 +381,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
> }
>
>+ chan->max_pkt_size = HVS_MAX_PKT_SIZE;
>+
premise, I don't know HyperV channels :-(
Is this change necessary to use hv_pkt_iter_first() instead of
hv_pkt_iter_first_raw()?
If yes, then please mention that you set this value in the commit
message, otherwise maybe better to have a separate patch.
Thanks,
Stefano
> ret = vmbus_open(chan, sndbuf, rcvbuf, NULL, 0, hvs_channel_cb,
> conn_from_host ? new : sk);
> if (ret != 0) {
>@@ -602,7 +607,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
> return -EOPNOTSUPP;
>
> if (need_refill) {
>- hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
>+ hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
> if (!hvs->recv_desc)
> return -ENOBUFS;
> ret = hvs_update_recv_data(hvs);
>@@ -618,7 +623,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
>
> hvs->recv_data_len -= to_read;
> if (hvs->recv_data_len == 0) {
>- hvs->recv_desc = hv_pkt_iter_next_raw(hvs->chan, hvs->recv_desc);
>+ hvs->recv_desc = hv_pkt_iter_next(hvs->chan, hvs->recv_desc);
> if (hvs->recv_desc) {
> ret = hvs_update_recv_data(hvs);
> if (ret)
>--
>2.25.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer
@ 2022-04-21 13:58 ` Stefano Garzarella
0 siblings, 0 replies; 26+ messages in thread
From: Stefano Garzarella @ 2022-04-21 13:58 UTC (permalink / raw)
To: Andrea Parri (Microsoft)
Cc: Wei Liu, Paolo Abeni, Stephen Hemminger, netdev, Haiyang Zhang,
Dexuan Cui, linux-hyperv, Michael Kelley, Jakub Kicinski,
virtualization, David Miller, linux-kernel
On Wed, Apr 20, 2022 at 10:07:17PM +0200, Andrea Parri (Microsoft) wrote:
>Pointers to VMbus packets sent by Hyper-V are used by the hv_sock driver
>within the guest VM. Hyper-V can send packets with erroneous values or
>modify packet fields after they are processed by the guest. To defend
>against these scenarios, copy the incoming packet after validating its
>length and offset fields using hv_pkt_iter_{first,next}(). In this way,
>the packet can no longer be modified by the host.
>
>Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
>---
> net/vmw_vsock/hyperv_transport.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 943352530936e..8c37d07017fc4 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -78,6 +78,9 @@ struct hvs_send_buf {
> ALIGN((payload_len), 8) + \
> VMBUS_PKT_TRAILER_SIZE)
>
>+/* Upper bound on the size of a VMbus packet for hv_sock */
>+#define HVS_MAX_PKT_SIZE HVS_PKT_LEN(HVS_MTU_SIZE)
>+
> union hvs_service_id {
> guid_t srv_id;
>
>@@ -378,6 +381,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
> }
>
>+ chan->max_pkt_size = HVS_MAX_PKT_SIZE;
>+
premise, I don't know HyperV channels :-(
Is this change necessary to use hv_pkt_iter_first() instead of
hv_pkt_iter_first_raw()?
If yes, then please mention that you set this value in the commit
message, otherwise maybe better to have a separate patch.
Thanks,
Stefano
> ret = vmbus_open(chan, sndbuf, rcvbuf, NULL, 0, hvs_channel_cb,
> conn_from_host ? new : sk);
> if (ret != 0) {
>@@ -602,7 +607,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
> return -EOPNOTSUPP;
>
> if (need_refill) {
>- hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
>+ hvs->recv_desc = hv_pkt_iter_first(hvs->chan);
> if (!hvs->recv_desc)
> return -ENOBUFS;
> ret = hvs_update_recv_data(hvs);
>@@ -618,7 +623,7 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk, struct msghdr *msg,
>
> hvs->recv_data_len -= to_read;
> if (hvs->recv_data_len == 0) {
>- hvs->recv_desc = hv_pkt_iter_next_raw(hvs->chan, hvs->recv_desc);
>+ hvs->recv_desc = hv_pkt_iter_next(hvs->chan, hvs->recv_desc);
> if (hvs->recv_desc) {
> ret = hvs_update_recv_data(hvs);
> if (ret)
>--
>2.25.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer
2022-04-21 13:58 ` Stefano Garzarella
(?)
@ 2022-04-21 15:21 ` Andrea Parri
-1 siblings, 0 replies; 26+ messages in thread
From: Andrea Parri @ 2022-04-21 15:21 UTC (permalink / raw)
To: Stefano Garzarella
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, David Miller, Jakub Kicinski,
Paolo Abeni, linux-hyperv, virtualization, netdev, linux-kernel
> > @@ -378,6 +381,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> > rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
> > }
> >
> > + chan->max_pkt_size = HVS_MAX_PKT_SIZE;
> > +
>
> premise, I don't know HyperV channels :-(
>
> Is this change necessary to use hv_pkt_iter_first() instead of
> hv_pkt_iter_first_raw()?
Yes, the change is required to initialize the buffer which holds the
copies of the incoming packets (in hv_ringbuffer_init()).
> If yes, then please mention that you set this value in the commit message,
> otherwise maybe better to have a separate patch.
Sure, will do.
Thanks,
Andrea
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values
2022-04-20 20:07 [PATCH 0/5] hv_sock: Hardening changes Andrea Parri (Microsoft)
2022-04-20 20:07 ` [PATCH 1/5] hv_sock: Check hv_pkt_iter_first_raw()'s return value Andrea Parri (Microsoft)
2022-04-20 20:07 ` [PATCH 2/5] hv_sock: Copy packets sent by Hyper-V out of the ring buffer Andrea Parri (Microsoft)
@ 2022-04-20 20:07 ` Andrea Parri (Microsoft)
2022-04-20 23:09 ` Michael Kelley (LINUX) via Virtualization
2022-04-21 14:08 ` Stefano Garzarella
2022-04-20 20:07 ` [PATCH 4/5] Drivers: hv: vmbus: Accept hv_sock offers in isolated guests Andrea Parri (Microsoft)
2022-04-20 20:07 ` [PATCH 5/5] Drivers: hv: vmbus: Refactor the ring-buffer iterator functions Andrea Parri (Microsoft)
4 siblings, 2 replies; 26+ messages in thread
From: Andrea Parri (Microsoft) @ 2022-04-20 20:07 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, Stefano Garzarella, David Miller,
Jakub Kicinski, Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel,
Andrea Parri (Microsoft)
For additional robustness in the face of Hyper-V errors or malicious
behavior, validate all values that originate from packets that Hyper-V
has sent to the guest in the host-to-guest ring buffer. Ensure that
invalid values cannot cause data being copied out of the bounds of the
source buffer in hvs_stream_dequeue().
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
include/linux/hyperv.h | 5 +++++
net/vmw_vsock/hyperv_transport.c | 11 +++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index fe2e0179ed51e..55478a6810b60 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1663,6 +1663,11 @@ static inline u32 hv_pkt_datalen(const struct vmpacket_descriptor *desc)
return (desc->len8 << 3) - (desc->offset8 << 3);
}
+/* Get packet length associated with descriptor */
+static inline u32 hv_pkt_len(const struct vmpacket_descriptor *desc)
+{
+ return desc->len8 << 3;
+}
struct vmpacket_descriptor *
hv_pkt_iter_first_raw(struct vmbus_channel *channel);
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 8c37d07017fc4..092cadc2c866d 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -577,12 +577,19 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
static int hvs_update_recv_data(struct hvsock *hvs)
{
struct hvs_recv_buf *recv_buf;
- u32 payload_len;
+ u32 pkt_len, payload_len;
+
+ pkt_len = hv_pkt_len(hvs->recv_desc);
+
+ /* Ensure the packet is big enough to read its header */
+ if (pkt_len < HVS_HEADER_LEN)
+ return -EIO;
recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
payload_len = recv_buf->hdr.data_size;
- if (payload_len > HVS_MTU_SIZE)
+ /* Ensure the packet is big enough to read its payload */
+ if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > HVS_MTU_SIZE)
return -EIO;
if (payload_len == 0)
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* RE: [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values
2022-04-20 20:07 ` [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values Andrea Parri (Microsoft)
@ 2022-04-20 23:09 ` Michael Kelley (LINUX) via Virtualization
2022-04-21 14:08 ` Stefano Garzarella
1 sibling, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) @ 2022-04-20 23:09 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> For additional robustness in the face of Hyper-V errors or malicious
> behavior, validate all values that originate from packets that Hyper-V
> has sent to the guest in the host-to-guest ring buffer. Ensure that
> invalid values cannot cause data being copied out of the bounds of the
> source buffer in hvs_stream_dequeue().
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> include/linux/hyperv.h | 5 +++++
> net/vmw_vsock/hyperv_transport.c | 11 +++++++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index fe2e0179ed51e..55478a6810b60 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1663,6 +1663,11 @@ static inline u32 hv_pkt_datalen(const struct
> vmpacket_descriptor *desc)
> return (desc->len8 << 3) - (desc->offset8 << 3);
> }
>
> +/* Get packet length associated with descriptor */
> +static inline u32 hv_pkt_len(const struct vmpacket_descriptor *desc)
> +{
> + return desc->len8 << 3;
> +}
>
> struct vmpacket_descriptor *
> hv_pkt_iter_first_raw(struct vmbus_channel *channel);
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index 8c37d07017fc4..092cadc2c866d 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -577,12 +577,19 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
> static int hvs_update_recv_data(struct hvsock *hvs)
> {
> struct hvs_recv_buf *recv_buf;
> - u32 payload_len;
> + u32 pkt_len, payload_len;
> +
> + pkt_len = hv_pkt_len(hvs->recv_desc);
> +
> + /* Ensure the packet is big enough to read its header */
> + if (pkt_len < HVS_HEADER_LEN)
> + return -EIO;
>
> recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
> payload_len = recv_buf->hdr.data_size;
>
> - if (payload_len > HVS_MTU_SIZE)
> + /* Ensure the packet is big enough to read its payload */
> + if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > HVS_MTU_SIZE)
> return -EIO;
>
> if (payload_len == 0)
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 26+ messages in thread* RE: [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values
@ 2022-04-20 23:09 ` Michael Kelley (LINUX) via Virtualization
0 siblings, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) via Virtualization @ 2022-04-20 23:09 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-hyperv, linux-kernel, virtualization
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> For additional robustness in the face of Hyper-V errors or malicious
> behavior, validate all values that originate from packets that Hyper-V
> has sent to the guest in the host-to-guest ring buffer. Ensure that
> invalid values cannot cause data being copied out of the bounds of the
> source buffer in hvs_stream_dequeue().
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> include/linux/hyperv.h | 5 +++++
> net/vmw_vsock/hyperv_transport.c | 11 +++++++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index fe2e0179ed51e..55478a6810b60 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1663,6 +1663,11 @@ static inline u32 hv_pkt_datalen(const struct
> vmpacket_descriptor *desc)
> return (desc->len8 << 3) - (desc->offset8 << 3);
> }
>
> +/* Get packet length associated with descriptor */
> +static inline u32 hv_pkt_len(const struct vmpacket_descriptor *desc)
> +{
> + return desc->len8 << 3;
> +}
>
> struct vmpacket_descriptor *
> hv_pkt_iter_first_raw(struct vmbus_channel *channel);
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index 8c37d07017fc4..092cadc2c866d 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -577,12 +577,19 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
> static int hvs_update_recv_data(struct hvsock *hvs)
> {
> struct hvs_recv_buf *recv_buf;
> - u32 payload_len;
> + u32 pkt_len, payload_len;
> +
> + pkt_len = hv_pkt_len(hvs->recv_desc);
> +
> + /* Ensure the packet is big enough to read its header */
> + if (pkt_len < HVS_HEADER_LEN)
> + return -EIO;
>
> recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
> payload_len = recv_buf->hdr.data_size;
>
> - if (payload_len > HVS_MTU_SIZE)
> + /* Ensure the packet is big enough to read its payload */
> + if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > HVS_MTU_SIZE)
> return -EIO;
>
> if (payload_len == 0)
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values
2022-04-20 20:07 ` [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values Andrea Parri (Microsoft)
@ 2022-04-21 14:08 ` Stefano Garzarella
2022-04-21 14:08 ` Stefano Garzarella
1 sibling, 0 replies; 26+ messages in thread
From: Stefano Garzarella @ 2022-04-21 14:08 UTC (permalink / raw)
To: Andrea Parri (Microsoft)
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, David Miller, Jakub Kicinski,
Paolo Abeni, linux-hyperv, virtualization, netdev, linux-kernel
On Wed, Apr 20, 2022 at 10:07:18PM +0200, Andrea Parri (Microsoft) wrote:
>For additional robustness in the face of Hyper-V errors or malicious
>behavior, validate all values that originate from packets that Hyper-V
>has sent to the guest in the host-to-guest ring buffer. Ensure that
>invalid values cannot cause data being copied out of the bounds of the
>source buffer in hvs_stream_dequeue().
>
>Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
>---
> include/linux/hyperv.h | 5 +++++
> net/vmw_vsock/hyperv_transport.c | 11 +++++++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
>index fe2e0179ed51e..55478a6810b60 100644
>--- a/include/linux/hyperv.h
>+++ b/include/linux/hyperv.h
>@@ -1663,6 +1663,11 @@ static inline u32 hv_pkt_datalen(const struct vmpacket_descriptor *desc)
> return (desc->len8 << 3) - (desc->offset8 << 3);
> }
>
>+/* Get packet length associated with descriptor */
>+static inline u32 hv_pkt_len(const struct vmpacket_descriptor *desc)
>+{
>+ return desc->len8 << 3;
>+}
>
> struct vmpacket_descriptor *
> hv_pkt_iter_first_raw(struct vmbus_channel *channel);
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 8c37d07017fc4..092cadc2c866d 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -577,12 +577,19 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
> static int hvs_update_recv_data(struct hvsock *hvs)
> {
> struct hvs_recv_buf *recv_buf;
>- u32 payload_len;
>+ u32 pkt_len, payload_len;
>+
>+ pkt_len = hv_pkt_len(hvs->recv_desc);
>+
>+ /* Ensure the packet is big enough to read its header */
>+ if (pkt_len < HVS_HEADER_LEN)
>+ return -EIO;
>
> recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
> payload_len = recv_buf->hdr.data_size;
>
>- if (payload_len > HVS_MTU_SIZE)
>+ /* Ensure the packet is big enough to read its payload */
>+ if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > HVS_MTU_SIZE)
checkpatch warns that we exceed 80 characters, I do not have a strong
opinion on this, but if you have to resend better break the condition
into 2 lines.
Maybe even update or remove the comment? (it only describes the first
condition, but the conditions are pretty clear, so I don't think it adds
much).
Thanks,
Stefano
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values
@ 2022-04-21 14:08 ` Stefano Garzarella
0 siblings, 0 replies; 26+ messages in thread
From: Stefano Garzarella @ 2022-04-21 14:08 UTC (permalink / raw)
To: Andrea Parri (Microsoft)
Cc: Wei Liu, Paolo Abeni, Stephen Hemminger, netdev, Haiyang Zhang,
Dexuan Cui, linux-hyperv, Michael Kelley, Jakub Kicinski,
virtualization, David Miller, linux-kernel
On Wed, Apr 20, 2022 at 10:07:18PM +0200, Andrea Parri (Microsoft) wrote:
>For additional robustness in the face of Hyper-V errors or malicious
>behavior, validate all values that originate from packets that Hyper-V
>has sent to the guest in the host-to-guest ring buffer. Ensure that
>invalid values cannot cause data being copied out of the bounds of the
>source buffer in hvs_stream_dequeue().
>
>Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
>---
> include/linux/hyperv.h | 5 +++++
> net/vmw_vsock/hyperv_transport.c | 11 +++++++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
>diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
>index fe2e0179ed51e..55478a6810b60 100644
>--- a/include/linux/hyperv.h
>+++ b/include/linux/hyperv.h
>@@ -1663,6 +1663,11 @@ static inline u32 hv_pkt_datalen(const struct vmpacket_descriptor *desc)
> return (desc->len8 << 3) - (desc->offset8 << 3);
> }
>
>+/* Get packet length associated with descriptor */
>+static inline u32 hv_pkt_len(const struct vmpacket_descriptor *desc)
>+{
>+ return desc->len8 << 3;
>+}
>
> struct vmpacket_descriptor *
> hv_pkt_iter_first_raw(struct vmbus_channel *channel);
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 8c37d07017fc4..092cadc2c866d 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -577,12 +577,19 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
> static int hvs_update_recv_data(struct hvsock *hvs)
> {
> struct hvs_recv_buf *recv_buf;
>- u32 payload_len;
>+ u32 pkt_len, payload_len;
>+
>+ pkt_len = hv_pkt_len(hvs->recv_desc);
>+
>+ /* Ensure the packet is big enough to read its header */
>+ if (pkt_len < HVS_HEADER_LEN)
>+ return -EIO;
>
> recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
> payload_len = recv_buf->hdr.data_size;
>
>- if (payload_len > HVS_MTU_SIZE)
>+ /* Ensure the packet is big enough to read its payload */
>+ if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > HVS_MTU_SIZE)
checkpatch warns that we exceed 80 characters, I do not have a strong
opinion on this, but if you have to resend better break the condition
into 2 lines.
Maybe even update or remove the comment? (it only describes the first
condition, but the conditions are pretty clear, so I don't think it adds
much).
Thanks,
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values
2022-04-21 14:08 ` Stefano Garzarella
(?)
@ 2022-04-21 15:30 ` Andrea Parri
2022-04-21 16:14 ` Stefano Garzarella
-1 siblings, 1 reply; 26+ messages in thread
From: Andrea Parri @ 2022-04-21 15:30 UTC (permalink / raw)
To: Stefano Garzarella
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, David Miller, Jakub Kicinski,
Paolo Abeni, linux-hyperv, virtualization, netdev, linux-kernel
> > @@ -577,12 +577,19 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
> > static int hvs_update_recv_data(struct hvsock *hvs)
> > {
> > struct hvs_recv_buf *recv_buf;
> > - u32 payload_len;
> > + u32 pkt_len, payload_len;
> > +
> > + pkt_len = hv_pkt_len(hvs->recv_desc);
> > +
> > + /* Ensure the packet is big enough to read its header */
> > + if (pkt_len < HVS_HEADER_LEN)
> > + return -EIO;
> >
> > recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
> > payload_len = recv_buf->hdr.data_size;
> >
> > - if (payload_len > HVS_MTU_SIZE)
> > + /* Ensure the packet is big enough to read its payload */
> > + if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > HVS_MTU_SIZE)
>
> checkpatch warns that we exceed 80 characters, I do not have a strong
> opinion on this, but if you have to resend better break the condition into 2
> lines.
Will break if preferred. (but does it really warn?? I understand that
the warning was deprecated and the "limit" increased to 100 chars...)
> Maybe even update or remove the comment? (it only describes the first
> condition, but the conditions are pretty clear, so I don't think it adds
> much).
Works for me. (taking it as this applies to the previous comment too.)
Thanks,
Andrea
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values
2022-04-21 15:30 ` Andrea Parri
@ 2022-04-21 16:14 ` Stefano Garzarella
0 siblings, 0 replies; 26+ messages in thread
From: Stefano Garzarella @ 2022-04-21 16:14 UTC (permalink / raw)
To: Andrea Parri
Cc: Wei Liu, Paolo Abeni, Stephen Hemminger, netdev, Haiyang Zhang,
Dexuan Cui, linux-hyperv, Michael Kelley, Jakub Kicinski,
Linux Virtualization, David Miller, kernel list
On Thu, Apr 21, 2022 at 5:30 PM Andrea Parri <parri.andrea@gmail.com> wrote:
>
> > > @@ -577,12 +577,19 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
> > > static int hvs_update_recv_data(struct hvsock *hvs)
> > > {
> > > struct hvs_recv_buf *recv_buf;
> > > - u32 payload_len;
> > > + u32 pkt_len, payload_len;
> > > +
> > > + pkt_len = hv_pkt_len(hvs->recv_desc);
> > > +
> > > + /* Ensure the packet is big enough to read its header */
> > > + if (pkt_len < HVS_HEADER_LEN)
> > > + return -EIO;
> > >
> > > recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
> > > payload_len = recv_buf->hdr.data_size;
> > >
> > > - if (payload_len > HVS_MTU_SIZE)
> > > + /* Ensure the packet is big enough to read its payload */
> > > + if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > HVS_MTU_SIZE)
> >
> > checkpatch warns that we exceed 80 characters, I do not have a strong
> > opinion on this, but if you have to resend better break the condition into 2
> > lines.
>
> Will break if preferred. (but does it really warn?? I understand that
> the warning was deprecated and the "limit" increased to 100 chars...)
I see the warn here:
https://patchwork.kernel.org/project/netdevbpf/patch/20220420200720.434717-4-parri.andrea@gmail.com/
in the kernel doc [1] we still say we prefer 80 columns, so I try to
follow, especially when it doesn't make things worse.
[1] https://docs.kernel.org/process/coding-style.html#breaking-long-lines-and-strings
>
>
> > Maybe even update or remove the comment? (it only describes the first
> > condition, but the conditions are pretty clear, so I don't think it adds
> > much).
>
> Works for me. (taking it as this applies to the previous comment too.)
Yep.
Thanks,
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values
@ 2022-04-21 16:14 ` Stefano Garzarella
0 siblings, 0 replies; 26+ messages in thread
From: Stefano Garzarella @ 2022-04-21 16:14 UTC (permalink / raw)
To: Andrea Parri
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, David Miller, Jakub Kicinski,
Paolo Abeni, linux-hyperv, Linux Virtualization, netdev,
kernel list
On Thu, Apr 21, 2022 at 5:30 PM Andrea Parri <parri.andrea@gmail.com> wrote:
>
> > > @@ -577,12 +577,19 @@ static bool hvs_dgram_allow(u32 cid, u32 port)
> > > static int hvs_update_recv_data(struct hvsock *hvs)
> > > {
> > > struct hvs_recv_buf *recv_buf;
> > > - u32 payload_len;
> > > + u32 pkt_len, payload_len;
> > > +
> > > + pkt_len = hv_pkt_len(hvs->recv_desc);
> > > +
> > > + /* Ensure the packet is big enough to read its header */
> > > + if (pkt_len < HVS_HEADER_LEN)
> > > + return -EIO;
> > >
> > > recv_buf = (struct hvs_recv_buf *)(hvs->recv_desc + 1);
> > > payload_len = recv_buf->hdr.data_size;
> > >
> > > - if (payload_len > HVS_MTU_SIZE)
> > > + /* Ensure the packet is big enough to read its payload */
> > > + if (payload_len > pkt_len - HVS_HEADER_LEN || payload_len > HVS_MTU_SIZE)
> >
> > checkpatch warns that we exceed 80 characters, I do not have a strong
> > opinion on this, but if you have to resend better break the condition into 2
> > lines.
>
> Will break if preferred. (but does it really warn?? I understand that
> the warning was deprecated and the "limit" increased to 100 chars...)
I see the warn here:
https://patchwork.kernel.org/project/netdevbpf/patch/20220420200720.434717-4-parri.andrea@gmail.com/
in the kernel doc [1] we still say we prefer 80 columns, so I try to
follow, especially when it doesn't make things worse.
[1] https://docs.kernel.org/process/coding-style.html#breaking-long-lines-and-strings
>
>
> > Maybe even update or remove the comment? (it only describes the first
> > condition, but the conditions are pretty clear, so I don't think it adds
> > much).
>
> Works for me. (taking it as this applies to the previous comment too.)
Yep.
Thanks,
Stefano
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 4/5] Drivers: hv: vmbus: Accept hv_sock offers in isolated guests
2022-04-20 20:07 [PATCH 0/5] hv_sock: Hardening changes Andrea Parri (Microsoft)
` (2 preceding siblings ...)
2022-04-20 20:07 ` [PATCH 3/5] hv_sock: Add validation for untrusted Hyper-V values Andrea Parri (Microsoft)
@ 2022-04-20 20:07 ` Andrea Parri (Microsoft)
2022-04-20 23:10 ` Michael Kelley (LINUX) via Virtualization
2022-04-20 20:07 ` [PATCH 5/5] Drivers: hv: vmbus: Refactor the ring-buffer iterator functions Andrea Parri (Microsoft)
4 siblings, 1 reply; 26+ messages in thread
From: Andrea Parri (Microsoft) @ 2022-04-20 20:07 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, Stefano Garzarella, David Miller,
Jakub Kicinski, Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel,
Andrea Parri (Microsoft)
So that isolated guests can communicate with the host via hv_sock
channels.
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
drivers/hv/channel_mgmt.c | 8 ++++++--
include/linux/hyperv.h | 8 ++++++--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 67be81208a2d9..d800220ee54f4 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -976,13 +976,17 @@ find_primary_channel_by_offer(const struct vmbus_channel_offer_channel *offer)
return channel;
}
-static bool vmbus_is_valid_device(const guid_t *guid)
+static bool vmbus_is_valid_offer(const struct vmbus_channel_offer_channel *offer)
{
+ const guid_t *guid = &offer->offer.if_type;
u16 i;
if (!hv_is_isolation_supported())
return true;
+ if (is_hvsock_offer(offer))
+ return true;
+
for (i = 0; i < ARRAY_SIZE(vmbus_devs); i++) {
if (guid_equal(guid, &vmbus_devs[i].guid))
return vmbus_devs[i].allowed_in_isolated;
@@ -1004,7 +1008,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
trace_vmbus_onoffer(offer);
- if (!vmbus_is_valid_device(&offer->offer.if_type)) {
+ if (!vmbus_is_valid_offer(offer)) {
pr_err_ratelimited("Invalid offer %d from the host supporting isolation\n",
offer->child_relid);
atomic_dec(&vmbus_connection.offer_in_progress);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 55478a6810b60..1112c5cf894e6 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1044,10 +1044,14 @@ struct vmbus_channel {
u64 vmbus_next_request_id(struct vmbus_channel *channel, u64 rqst_addr);
u64 vmbus_request_addr(struct vmbus_channel *channel, u64 trans_id);
+static inline bool is_hvsock_offer(const struct vmbus_channel_offer_channel *o)
+{
+ return !!(o->offer.chn_flags & VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
+}
+
static inline bool is_hvsock_channel(const struct vmbus_channel *c)
{
- return !!(c->offermsg.offer.chn_flags &
- VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
+ return is_hvsock_offer(&c->offermsg);
}
static inline bool is_sub_channel(const struct vmbus_channel *c)
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* RE: [PATCH 4/5] Drivers: hv: vmbus: Accept hv_sock offers in isolated guests
2022-04-20 20:07 ` [PATCH 4/5] Drivers: hv: vmbus: Accept hv_sock offers in isolated guests Andrea Parri (Microsoft)
@ 2022-04-20 23:10 ` Michael Kelley (LINUX) via Virtualization
0 siblings, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) @ 2022-04-20 23:10 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> So that isolated guests can communicate with the host via hv_sock
> channels.
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> drivers/hv/channel_mgmt.c | 8 ++++++--
> include/linux/hyperv.h | 8 ++++++--
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 67be81208a2d9..d800220ee54f4 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -976,13 +976,17 @@ find_primary_channel_by_offer(const struct
> vmbus_channel_offer_channel *offer)
> return channel;
> }
>
> -static bool vmbus_is_valid_device(const guid_t *guid)
> +static bool vmbus_is_valid_offer(const struct vmbus_channel_offer_channel *offer)
> {
> + const guid_t *guid = &offer->offer.if_type;
> u16 i;
>
> if (!hv_is_isolation_supported())
> return true;
>
> + if (is_hvsock_offer(offer))
> + return true;
> +
> for (i = 0; i < ARRAY_SIZE(vmbus_devs); i++) {
> if (guid_equal(guid, &vmbus_devs[i].guid))
> return vmbus_devs[i].allowed_in_isolated;
> @@ -1004,7 +1008,7 @@ static void vmbus_onoffer(struct
> vmbus_channel_message_header *hdr)
>
> trace_vmbus_onoffer(offer);
>
> - if (!vmbus_is_valid_device(&offer->offer.if_type)) {
> + if (!vmbus_is_valid_offer(offer)) {
> pr_err_ratelimited("Invalid offer %d from the host supporting
> isolation\n",
> offer->child_relid);
> atomic_dec(&vmbus_connection.offer_in_progress);
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 55478a6810b60..1112c5cf894e6 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1044,10 +1044,14 @@ struct vmbus_channel {
> u64 vmbus_next_request_id(struct vmbus_channel *channel, u64 rqst_addr);
> u64 vmbus_request_addr(struct vmbus_channel *channel, u64 trans_id);
>
> +static inline bool is_hvsock_offer(const struct vmbus_channel_offer_channel *o)
> +{
> + return !!(o->offer.chn_flags & VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
> +}
> +
> static inline bool is_hvsock_channel(const struct vmbus_channel *c)
> {
> - return !!(c->offermsg.offer.chn_flags &
> - VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
> + return is_hvsock_offer(&c->offermsg);
> }
>
> static inline bool is_sub_channel(const struct vmbus_channel *c)
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 26+ messages in thread* RE: [PATCH 4/5] Drivers: hv: vmbus: Accept hv_sock offers in isolated guests
@ 2022-04-20 23:10 ` Michael Kelley (LINUX) via Virtualization
0 siblings, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) via Virtualization @ 2022-04-20 23:10 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-hyperv, linux-kernel, virtualization
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> So that isolated guests can communicate with the host via hv_sock
> channels.
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> drivers/hv/channel_mgmt.c | 8 ++++++--
> include/linux/hyperv.h | 8 ++++++--
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 67be81208a2d9..d800220ee54f4 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -976,13 +976,17 @@ find_primary_channel_by_offer(const struct
> vmbus_channel_offer_channel *offer)
> return channel;
> }
>
> -static bool vmbus_is_valid_device(const guid_t *guid)
> +static bool vmbus_is_valid_offer(const struct vmbus_channel_offer_channel *offer)
> {
> + const guid_t *guid = &offer->offer.if_type;
> u16 i;
>
> if (!hv_is_isolation_supported())
> return true;
>
> + if (is_hvsock_offer(offer))
> + return true;
> +
> for (i = 0; i < ARRAY_SIZE(vmbus_devs); i++) {
> if (guid_equal(guid, &vmbus_devs[i].guid))
> return vmbus_devs[i].allowed_in_isolated;
> @@ -1004,7 +1008,7 @@ static void vmbus_onoffer(struct
> vmbus_channel_message_header *hdr)
>
> trace_vmbus_onoffer(offer);
>
> - if (!vmbus_is_valid_device(&offer->offer.if_type)) {
> + if (!vmbus_is_valid_offer(offer)) {
> pr_err_ratelimited("Invalid offer %d from the host supporting
> isolation\n",
> offer->child_relid);
> atomic_dec(&vmbus_connection.offer_in_progress);
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 55478a6810b60..1112c5cf894e6 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1044,10 +1044,14 @@ struct vmbus_channel {
> u64 vmbus_next_request_id(struct vmbus_channel *channel, u64 rqst_addr);
> u64 vmbus_request_addr(struct vmbus_channel *channel, u64 trans_id);
>
> +static inline bool is_hvsock_offer(const struct vmbus_channel_offer_channel *o)
> +{
> + return !!(o->offer.chn_flags & VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
> +}
> +
> static inline bool is_hvsock_channel(const struct vmbus_channel *c)
> {
> - return !!(c->offermsg.offer.chn_flags &
> - VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
> + return is_hvsock_offer(&c->offermsg);
> }
>
> static inline bool is_sub_channel(const struct vmbus_channel *c)
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 5/5] Drivers: hv: vmbus: Refactor the ring-buffer iterator functions
2022-04-20 20:07 [PATCH 0/5] hv_sock: Hardening changes Andrea Parri (Microsoft)
` (3 preceding siblings ...)
2022-04-20 20:07 ` [PATCH 4/5] Drivers: hv: vmbus: Accept hv_sock offers in isolated guests Andrea Parri (Microsoft)
@ 2022-04-20 20:07 ` Andrea Parri (Microsoft)
2022-04-20 23:15 ` Michael Kelley (LINUX) via Virtualization
4 siblings, 1 reply; 26+ messages in thread
From: Andrea Parri (Microsoft) @ 2022-04-20 20:07 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Michael Kelley, Stefano Garzarella, David Miller,
Jakub Kicinski, Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel,
Andrea Parri (Microsoft)
With no users of hv_pkt_iter_next_raw() and no "external" users of
hv_pkt_iter_first_raw(), the iterator functions can be refactored
and simplified to remove some indirection/code.
Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
---
drivers/hv/ring_buffer.c | 32 +++++++++-----------------------
include/linux/hyperv.h | 35 ++++-------------------------------
2 files changed, 13 insertions(+), 54 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 3d215d9dec433..fa98b3a91206a 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -421,7 +421,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
memcpy(buffer, (const char *)desc + offset, packetlen);
/* Advance ring index to next packet descriptor */
- __hv_pkt_iter_next(channel, desc, true);
+ __hv_pkt_iter_next(channel, desc);
/* Notify host of update */
hv_pkt_iter_close(channel);
@@ -456,22 +456,6 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info *rbi)
return (rbi->ring_datasize - priv_read_loc) + write_loc;
}
-/*
- * Get first vmbus packet without copying it out of the ring buffer
- */
-struct vmpacket_descriptor *hv_pkt_iter_first_raw(struct vmbus_channel *channel)
-{
- struct hv_ring_buffer_info *rbi = &channel->inbound;
-
- hv_debug_delay_test(channel, MESSAGE_DELAY);
-
- if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
- return NULL;
-
- return (struct vmpacket_descriptor *)(hv_get_ring_buffer(rbi) + rbi->priv_read_index);
-}
-EXPORT_SYMBOL_GPL(hv_pkt_iter_first_raw);
-
/*
* Get first vmbus packet from ring buffer after read_index
*
@@ -483,11 +467,14 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct vmbus_channel *channel)
struct vmpacket_descriptor *desc, *desc_copy;
u32 bytes_avail, pkt_len, pkt_offset;
- desc = hv_pkt_iter_first_raw(channel);
- if (!desc)
+ hv_debug_delay_test(channel, MESSAGE_DELAY);
+
+ bytes_avail = hv_pkt_iter_avail(rbi);
+ if (bytes_avail < sizeof(struct vmpacket_descriptor))
return NULL;
+ bytes_avail = min(rbi->pkt_buffer_size, bytes_avail);
- bytes_avail = min(rbi->pkt_buffer_size, hv_pkt_iter_avail(rbi));
+ desc = (struct vmpacket_descriptor *)(hv_get_ring_buffer(rbi) + rbi->priv_read_index);
/*
* Ensure the compiler does not use references to incoming Hyper-V values (which
@@ -534,8 +521,7 @@ EXPORT_SYMBOL_GPL(hv_pkt_iter_first);
*/
struct vmpacket_descriptor *
__hv_pkt_iter_next(struct vmbus_channel *channel,
- const struct vmpacket_descriptor *desc,
- bool copy)
+ const struct vmpacket_descriptor *desc)
{
struct hv_ring_buffer_info *rbi = &channel->inbound;
u32 packetlen = desc->len8 << 3;
@@ -548,7 +534,7 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
rbi->priv_read_index -= dsize;
/* more data? */
- return copy ? hv_pkt_iter_first(channel) : hv_pkt_iter_first_raw(channel);
+ return hv_pkt_iter_first(channel);
}
EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 1112c5cf894e6..370adc9971d3e 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1673,55 +1673,28 @@ static inline u32 hv_pkt_len(const struct vmpacket_descriptor *desc)
return desc->len8 << 3;
}
-struct vmpacket_descriptor *
-hv_pkt_iter_first_raw(struct vmbus_channel *channel);
-
struct vmpacket_descriptor *
hv_pkt_iter_first(struct vmbus_channel *channel);
struct vmpacket_descriptor *
__hv_pkt_iter_next(struct vmbus_channel *channel,
- const struct vmpacket_descriptor *pkt,
- bool copy);
+ const struct vmpacket_descriptor *pkt);
void hv_pkt_iter_close(struct vmbus_channel *channel);
static inline struct vmpacket_descriptor *
-hv_pkt_iter_next_pkt(struct vmbus_channel *channel,
- const struct vmpacket_descriptor *pkt,
- bool copy)
+hv_pkt_iter_next(struct vmbus_channel *channel,
+ const struct vmpacket_descriptor *pkt)
{
struct vmpacket_descriptor *nxt;
- nxt = __hv_pkt_iter_next(channel, pkt, copy);
+ nxt = __hv_pkt_iter_next(channel, pkt);
if (!nxt)
hv_pkt_iter_close(channel);
return nxt;
}
-/*
- * Get next packet descriptor without copying it out of the ring buffer
- * If at end of list, return NULL and update host.
- */
-static inline struct vmpacket_descriptor *
-hv_pkt_iter_next_raw(struct vmbus_channel *channel,
- const struct vmpacket_descriptor *pkt)
-{
- return hv_pkt_iter_next_pkt(channel, pkt, false);
-}
-
-/*
- * Get next packet descriptor from iterator
- * If at end of list, return NULL and update host.
- */
-static inline struct vmpacket_descriptor *
-hv_pkt_iter_next(struct vmbus_channel *channel,
- const struct vmpacket_descriptor *pkt)
-{
- return hv_pkt_iter_next_pkt(channel, pkt, true);
-}
-
#define foreach_vmbus_pkt(pkt, channel) \
for (pkt = hv_pkt_iter_first(channel); pkt; \
pkt = hv_pkt_iter_next(channel, pkt))
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* RE: [PATCH 5/5] Drivers: hv: vmbus: Refactor the ring-buffer iterator functions
2022-04-20 20:07 ` [PATCH 5/5] Drivers: hv: vmbus: Refactor the ring-buffer iterator functions Andrea Parri (Microsoft)
@ 2022-04-20 23:15 ` Michael Kelley (LINUX) via Virtualization
0 siblings, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) @ 2022-04-20 23:15 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: linux-hyperv, virtualization, netdev, linux-kernel
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> With no users of hv_pkt_iter_next_raw() and no "external" users of
> hv_pkt_iter_first_raw(), the iterator functions can be refactored
> and simplified to remove some indirection/code.
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> drivers/hv/ring_buffer.c | 32 +++++++++-----------------------
> include/linux/hyperv.h | 35 ++++-------------------------------
> 2 files changed, 13 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 3d215d9dec433..fa98b3a91206a 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -421,7 +421,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
> memcpy(buffer, (const char *)desc + offset, packetlen);
>
> /* Advance ring index to next packet descriptor */
> - __hv_pkt_iter_next(channel, desc, true);
> + __hv_pkt_iter_next(channel, desc);
>
> /* Notify host of update */
> hv_pkt_iter_close(channel);
> @@ -456,22 +456,6 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info
> *rbi)
> return (rbi->ring_datasize - priv_read_loc) + write_loc;
> }
>
> -/*
> - * Get first vmbus packet without copying it out of the ring buffer
> - */
> -struct vmpacket_descriptor *hv_pkt_iter_first_raw(struct vmbus_channel *channel)
> -{
> - struct hv_ring_buffer_info *rbi = &channel->inbound;
> -
> - hv_debug_delay_test(channel, MESSAGE_DELAY);
> -
> - if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
> - return NULL;
> -
> - return (struct vmpacket_descriptor *)(hv_get_ring_buffer(rbi) + rbi-
> >priv_read_index);
> -}
> -EXPORT_SYMBOL_GPL(hv_pkt_iter_first_raw);
> -
> /*
> * Get first vmbus packet from ring buffer after read_index
> *
> @@ -483,11 +467,14 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct
> vmbus_channel *channel)
> struct vmpacket_descriptor *desc, *desc_copy;
> u32 bytes_avail, pkt_len, pkt_offset;
>
> - desc = hv_pkt_iter_first_raw(channel);
> - if (!desc)
> + hv_debug_delay_test(channel, MESSAGE_DELAY);
> +
> + bytes_avail = hv_pkt_iter_avail(rbi);
> + if (bytes_avail < sizeof(struct vmpacket_descriptor))
> return NULL;
> + bytes_avail = min(rbi->pkt_buffer_size, bytes_avail);
>
> - bytes_avail = min(rbi->pkt_buffer_size, hv_pkt_iter_avail(rbi));
> + desc = (struct vmpacket_descriptor *)(hv_get_ring_buffer(rbi) + rbi-
> >priv_read_index);
>
> /*
> * Ensure the compiler does not use references to incoming Hyper-V values
> (which
> @@ -534,8 +521,7 @@ EXPORT_SYMBOL_GPL(hv_pkt_iter_first);
> */
> struct vmpacket_descriptor *
> __hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *desc,
> - bool copy)
> + const struct vmpacket_descriptor *desc)
> {
> struct hv_ring_buffer_info *rbi = &channel->inbound;
> u32 packetlen = desc->len8 << 3;
> @@ -548,7 +534,7 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
> rbi->priv_read_index -= dsize;
>
> /* more data? */
> - return copy ? hv_pkt_iter_first(channel) : hv_pkt_iter_first_raw(channel);
> + return hv_pkt_iter_first(channel);
> }
> EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
>
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 1112c5cf894e6..370adc9971d3e 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1673,55 +1673,28 @@ static inline u32 hv_pkt_len(const struct
> vmpacket_descriptor *desc)
> return desc->len8 << 3;
> }
>
> -struct vmpacket_descriptor *
> -hv_pkt_iter_first_raw(struct vmbus_channel *channel);
> -
> struct vmpacket_descriptor *
> hv_pkt_iter_first(struct vmbus_channel *channel);
>
> struct vmpacket_descriptor *
> __hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt,
> - bool copy);
> + const struct vmpacket_descriptor *pkt);
>
> void hv_pkt_iter_close(struct vmbus_channel *channel);
>
> static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next_pkt(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt,
> - bool copy)
> +hv_pkt_iter_next(struct vmbus_channel *channel,
> + const struct vmpacket_descriptor *pkt)
> {
> struct vmpacket_descriptor *nxt;
>
> - nxt = __hv_pkt_iter_next(channel, pkt, copy);
> + nxt = __hv_pkt_iter_next(channel, pkt);
> if (!nxt)
> hv_pkt_iter_close(channel);
>
> return nxt;
> }
>
> -/*
> - * Get next packet descriptor without copying it out of the ring buffer
> - * If at end of list, return NULL and update host.
> - */
> -static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next_raw(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt)
> -{
> - return hv_pkt_iter_next_pkt(channel, pkt, false);
> -}
> -
> -/*
> - * Get next packet descriptor from iterator
> - * If at end of list, return NULL and update host.
> - */
> -static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt)
> -{
> - return hv_pkt_iter_next_pkt(channel, pkt, true);
> -}
> -
> #define foreach_vmbus_pkt(pkt, channel) \
> for (pkt = hv_pkt_iter_first(channel); pkt; \
> pkt = hv_pkt_iter_next(channel, pkt))
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 26+ messages in thread* RE: [PATCH 5/5] Drivers: hv: vmbus: Refactor the ring-buffer iterator functions
@ 2022-04-20 23:15 ` Michael Kelley (LINUX) via Virtualization
0 siblings, 0 replies; 26+ messages in thread
From: Michael Kelley (LINUX) via Virtualization @ 2022-04-20 23:15 UTC (permalink / raw)
To: Andrea Parri (Microsoft),
KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
Dexuan Cui, Stefano Garzarella, David Miller, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linux-hyperv, linux-kernel, virtualization
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, April 20, 2022 1:07 PM
>
> With no users of hv_pkt_iter_next_raw() and no "external" users of
> hv_pkt_iter_first_raw(), the iterator functions can be refactored
> and simplified to remove some indirection/code.
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> drivers/hv/ring_buffer.c | 32 +++++++++-----------------------
> include/linux/hyperv.h | 35 ++++-------------------------------
> 2 files changed, 13 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 3d215d9dec433..fa98b3a91206a 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -421,7 +421,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
> memcpy(buffer, (const char *)desc + offset, packetlen);
>
> /* Advance ring index to next packet descriptor */
> - __hv_pkt_iter_next(channel, desc, true);
> + __hv_pkt_iter_next(channel, desc);
>
> /* Notify host of update */
> hv_pkt_iter_close(channel);
> @@ -456,22 +456,6 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info
> *rbi)
> return (rbi->ring_datasize - priv_read_loc) + write_loc;
> }
>
> -/*
> - * Get first vmbus packet without copying it out of the ring buffer
> - */
> -struct vmpacket_descriptor *hv_pkt_iter_first_raw(struct vmbus_channel *channel)
> -{
> - struct hv_ring_buffer_info *rbi = &channel->inbound;
> -
> - hv_debug_delay_test(channel, MESSAGE_DELAY);
> -
> - if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
> - return NULL;
> -
> - return (struct vmpacket_descriptor *)(hv_get_ring_buffer(rbi) + rbi-
> >priv_read_index);
> -}
> -EXPORT_SYMBOL_GPL(hv_pkt_iter_first_raw);
> -
> /*
> * Get first vmbus packet from ring buffer after read_index
> *
> @@ -483,11 +467,14 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct
> vmbus_channel *channel)
> struct vmpacket_descriptor *desc, *desc_copy;
> u32 bytes_avail, pkt_len, pkt_offset;
>
> - desc = hv_pkt_iter_first_raw(channel);
> - if (!desc)
> + hv_debug_delay_test(channel, MESSAGE_DELAY);
> +
> + bytes_avail = hv_pkt_iter_avail(rbi);
> + if (bytes_avail < sizeof(struct vmpacket_descriptor))
> return NULL;
> + bytes_avail = min(rbi->pkt_buffer_size, bytes_avail);
>
> - bytes_avail = min(rbi->pkt_buffer_size, hv_pkt_iter_avail(rbi));
> + desc = (struct vmpacket_descriptor *)(hv_get_ring_buffer(rbi) + rbi-
> >priv_read_index);
>
> /*
> * Ensure the compiler does not use references to incoming Hyper-V values
> (which
> @@ -534,8 +521,7 @@ EXPORT_SYMBOL_GPL(hv_pkt_iter_first);
> */
> struct vmpacket_descriptor *
> __hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *desc,
> - bool copy)
> + const struct vmpacket_descriptor *desc)
> {
> struct hv_ring_buffer_info *rbi = &channel->inbound;
> u32 packetlen = desc->len8 << 3;
> @@ -548,7 +534,7 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
> rbi->priv_read_index -= dsize;
>
> /* more data? */
> - return copy ? hv_pkt_iter_first(channel) : hv_pkt_iter_first_raw(channel);
> + return hv_pkt_iter_first(channel);
> }
> EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
>
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 1112c5cf894e6..370adc9971d3e 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1673,55 +1673,28 @@ static inline u32 hv_pkt_len(const struct
> vmpacket_descriptor *desc)
> return desc->len8 << 3;
> }
>
> -struct vmpacket_descriptor *
> -hv_pkt_iter_first_raw(struct vmbus_channel *channel);
> -
> struct vmpacket_descriptor *
> hv_pkt_iter_first(struct vmbus_channel *channel);
>
> struct vmpacket_descriptor *
> __hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt,
> - bool copy);
> + const struct vmpacket_descriptor *pkt);
>
> void hv_pkt_iter_close(struct vmbus_channel *channel);
>
> static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next_pkt(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt,
> - bool copy)
> +hv_pkt_iter_next(struct vmbus_channel *channel,
> + const struct vmpacket_descriptor *pkt)
> {
> struct vmpacket_descriptor *nxt;
>
> - nxt = __hv_pkt_iter_next(channel, pkt, copy);
> + nxt = __hv_pkt_iter_next(channel, pkt);
> if (!nxt)
> hv_pkt_iter_close(channel);
>
> return nxt;
> }
>
> -/*
> - * Get next packet descriptor without copying it out of the ring buffer
> - * If at end of list, return NULL and update host.
> - */
> -static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next_raw(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt)
> -{
> - return hv_pkt_iter_next_pkt(channel, pkt, false);
> -}
> -
> -/*
> - * Get next packet descriptor from iterator
> - * If at end of list, return NULL and update host.
> - */
> -static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt)
> -{
> - return hv_pkt_iter_next_pkt(channel, pkt, true);
> -}
> -
> #define foreach_vmbus_pkt(pkt, channel) \
> for (pkt = hv_pkt_iter_first(channel); pkt; \
> pkt = hv_pkt_iter_next(channel, pkt))
> --
> 2.25.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 26+ messages in thread