| Message ID | 20210508163544.3432132-1-arseny.krasnov@kaspersky.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio/vsock: introduce SOCK_SEQPACKET support | expand |
On Sat, May 08, 2021 at 07:35:40PM +0300, Arseny Krasnov wrote: >This modifies current receive logic for SEQPACKET support: >1) Inserts 'RW' packet to socket's rx queue, but without merging with > buffer of last packet in queue. This is not true anymore, right? >2) Performs check for packet and socket types on receive(if mismatch, > then reset connection). > >Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> >Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Also this patch is changed :-) >--- > net/vmw_vsock/virtio_transport_common.c | 28 +++++++++++++++++++++++-- > 1 file changed, 26 insertions(+), 2 deletions(-) > >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >index f649a21dd23b..7fea0a2192f7 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -165,6 +165,14 @@ void virtio_transport_deliver_tap_pkt(struct virtio_vsock_pkt *pkt) > } > EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt); > >+static u16 virtio_transport_get_type(struct sock *sk) >+{ >+ if (sk->sk_type == SOCK_STREAM) >+ return VIRTIO_VSOCK_TYPE_STREAM; >+ else >+ return VIRTIO_VSOCK_TYPE_SEQPACKET; >+} >+ > /* This function can only be used on connecting/connected sockets, > * since a socket assigned to a transport is required. > * >@@ -980,11 +988,15 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, > /* If there is space in the last packet queued, we copy the > * new packet in its buffer. > */ >- if (pkt->len <= last_pkt->buf_len - last_pkt->len) { >+ if ((pkt->len <= last_pkt->buf_len - last_pkt->len) && >+ !(le32_to_cpu(last_pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR)) { Maybe we should update the comment above. > memcpy(last_pkt->buf + last_pkt->len, pkt->buf, > pkt->len); > last_pkt->len += pkt->len; > free_pkt = true; >+ >+ if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR) >+ last_pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR); What about doing the following in any case? last_pkt->hdr.flags |= pkt->hdr.flags; > goto out; > } > }
On 13.05.2021 15:14, Stefano Garzarella wrote: > On Sat, May 08, 2021 at 07:35:40PM +0300, Arseny Krasnov wrote: >> This modifies current receive logic for SEQPACKET support: >> 1) Inserts 'RW' packet to socket's rx queue, but without merging with >> buffer of last packet in queue. > This is not true anymore, right? > >> 2) Performs check for packet and socket types on receive(if mismatch, >> then reset connection). >> >> Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com> >> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > Also this patch is changed :-) > >> --- >> net/vmw_vsock/virtio_transport_common.c | 28 +++++++++++++++++++++++-- >> 1 file changed, 26 insertions(+), 2 deletions(-) >> >> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >> index f649a21dd23b..7fea0a2192f7 100644 >> --- a/net/vmw_vsock/virtio_transport_common.c >> +++ b/net/vmw_vsock/virtio_transport_common.c >> @@ -165,6 +165,14 @@ void virtio_transport_deliver_tap_pkt(struct virtio_vsock_pkt *pkt) >> } >> EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt); >> >> +static u16 virtio_transport_get_type(struct sock *sk) >> +{ >> + if (sk->sk_type == SOCK_STREAM) >> + return VIRTIO_VSOCK_TYPE_STREAM; >> + else >> + return VIRTIO_VSOCK_TYPE_SEQPACKET; >> +} >> + >> /* This function can only be used on connecting/connected sockets, >> * since a socket assigned to a transport is required. >> * >> @@ -980,11 +988,15 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, >> /* If there is space in the last packet queued, we copy the >> * new packet in its buffer. >> */ >> - if (pkt->len <= last_pkt->buf_len - last_pkt->len) { >> + if ((pkt->len <= last_pkt->buf_len - last_pkt->len) && >> + !(le32_to_cpu(last_pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR)) { > Maybe we should update the comment above. > >> memcpy(last_pkt->buf + last_pkt->len, pkt->buf, >> pkt->len); >> last_pkt->len += pkt->len; >> free_pkt = true; >> + >> + if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR) >> + last_pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR); > What about doing the following in any case? > > last_pkt->hdr.flags |= pkt->hdr.flags; Ack > >> goto out; >> } >> } >
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index f649a21dd23b..7fea0a2192f7 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -165,6 +165,14 @@ void virtio_transport_deliver_tap_pkt(struct virtio_vsock_pkt *pkt) } EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt); +static u16 virtio_transport_get_type(struct sock *sk) +{ + if (sk->sk_type == SOCK_STREAM) + return VIRTIO_VSOCK_TYPE_STREAM; + else + return VIRTIO_VSOCK_TYPE_SEQPACKET; +} + /* This function can only be used on connecting/connected sockets, * since a socket assigned to a transport is required. * @@ -980,11 +988,15 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk, /* If there is space in the last packet queued, we copy the * new packet in its buffer. */ - if (pkt->len <= last_pkt->buf_len - last_pkt->len) { + if ((pkt->len <= last_pkt->buf_len - last_pkt->len) && + !(le32_to_cpu(last_pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR)) { memcpy(last_pkt->buf + last_pkt->len, pkt->buf, pkt->len); last_pkt->len += pkt->len; free_pkt = true; + + if (le32_to_cpu(pkt->hdr.flags) & VIRTIO_VSOCK_SEQ_EOR) + last_pkt->hdr.flags |= cpu_to_le32(VIRTIO_VSOCK_SEQ_EOR); goto out; } } @@ -1150,6 +1162,12 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt, return 0; } +static bool virtio_transport_valid_type(u16 type) +{ + return (type == VIRTIO_VSOCK_TYPE_STREAM) || + (type == VIRTIO_VSOCK_TYPE_SEQPACKET); +} + /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex * lock. */ @@ -1175,7 +1193,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t, le32_to_cpu(pkt->hdr.buf_alloc), le32_to_cpu(pkt->hdr.fwd_cnt)); - if (le16_to_cpu(pkt->hdr.type) != VIRTIO_VSOCK_TYPE_STREAM) { + if (!virtio_transport_valid_type(le16_to_cpu(pkt->hdr.type))) { (void)virtio_transport_reset_no_sock(t, pkt); goto free_pkt; } @@ -1192,6 +1210,12 @@ void virtio_transport_recv_pkt(struct virtio_transport *t, } } + if (virtio_transport_get_type(sk) != le16_to_cpu(pkt->hdr.type)) { + (void)virtio_transport_reset_no_sock(t, pkt); + sock_put(sk); + goto free_pkt; + } + vsk = vsock_sk(sk); lock_sock(sk);