| Message ID | b6fe000f-5638-28d0-525f-ce38cc2cb036@sberdevices.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio/vsock: fix credit update logic | expand |
On 04.03.2023 02:00, Robert Eshleman . wrote: > On Fri, Mar 3, 2023 at 2:05 PM Arseniy Krasnov <avkrasnov@sberdevices.ru> > wrote: > >> In case of SOCK_SEQPACKET all sk_buffs are used once - after read some >> data from it, it will be removed, so user will never read rest of the >> data. Thus we need to update credit parameters of the socket like whole >> sk_buff is read - so call 'skb_pull()' for the whole buffer. >> >> Fixes: 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff") >> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> >> --- >> net/vmw_vsock/virtio_transport_common.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/vmw_vsock/virtio_transport_common.c >> b/net/vmw_vsock/virtio_transport_common.c >> index d80075e1db42..bbcf331b6ad6 100644 >> --- a/net/vmw_vsock/virtio_transport_common.c >> +++ b/net/vmw_vsock/virtio_transport_common.c >> @@ -470,7 +470,7 @@ static int >> virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk, >> dequeued_len = err; >> } else { >> user_buf_len -= bytes_to_copy; >> - skb_pull(skb, bytes_to_copy); >> + skb_pull(skb, skb->len); >> } >> >> > I believe this may also need to be done when memcpy_to_msg() returns an > error. Hello! Thanks for quick reply. Yes, moreover in case of SEQPACKET 'skb_pull()' must be called every time when skbuff was removed from queue - it doesn't matter did we copy data from, get error on memcpy_to_msg(), or just drop it - otherwise we get leak of 'rx_bytes'. Also in case of STREAM, skb_pull() must be called for the rest of data in skbuff in case of error, because again - 'rx_bytes' will leak. I think, i'll prepare fixes and tests for this case in the next week Thanks, Arseniy > > Best, > Bobby >
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index d80075e1db42..bbcf331b6ad6 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -470,7 +470,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk, dequeued_len = err; } else { user_buf_len -= bytes_to_copy; - skb_pull(skb, bytes_to_copy); + skb_pull(skb, skb->len); } spin_lock_bh(&vvs->rx_lock);
In case of SOCK_SEQPACKET all sk_buffs are used once - after read some data from it, it will be removed, so user will never read rest of the data. Thus we need to update credit parameters of the socket like whole sk_buff is read - so call 'skb_pull()' for the whole buffer. Fixes: 71dc9ec9ac7d ("virtio/vsock: replace virtio_vsock_pkt with sk_buff") Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru> --- net/vmw_vsock/virtio_transport_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)