* [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions
@ 2019-12-13 18:47 Stefano Garzarella
2019-12-13 18:48 ` [PATCH net 1/2] vsock/virtio: fix null-pointer dereference in virtio_transport_recv_listen() Stefano Garzarella
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Stefano Garzarella @ 2019-12-13 18:47 UTC (permalink / raw)
To: davem
Cc: Stefano Garzarella, linux-kernel, netdev, kvm, virtualization,
Stefan Hajnoczi
This series mainly solves a possible null-pointer dereference in
virtio_transport_recv_listen() introduced with the multi-transport
support [PATCH 1].
PATCH 2 adds a WARN_ON check for the same potential issue
and a returned error in the virtio_transport_send_pkt_info() function
to avoid crashing the kernel.
Stefano Garzarella (2):
vsock/virtio: fix null-pointer dereference in
virtio_transport_recv_listen()
vsock/virtio: add WARN_ON check on virtio_transport_get_ops()
net/vmw_vsock/virtio_transport_common.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH net 1/2] vsock/virtio: fix null-pointer dereference in virtio_transport_recv_listen()
2019-12-13 18:47 [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions Stefano Garzarella
@ 2019-12-13 18:48 ` Stefano Garzarella
2019-12-13 18:48 ` [PATCH net 2/2] vsock/virtio: add WARN_ON check on virtio_transport_get_ops() Stefano Garzarella
2019-12-17 0:07 ` [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Stefano Garzarella @ 2019-12-13 18:48 UTC (permalink / raw)
To: davem
Cc: Stefano Garzarella, linux-kernel, netdev, kvm, virtualization,
Stefan Hajnoczi
With multi-transport support, listener sockets are not bound to any
transport. So, calling virtio_transport_reset(), when an error
occurs, on a listener socket produces the following null-pointer
dereference:
BUG: kernel NULL pointer dereference, address: 00000000000000e8
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 0 PID: 20 Comm: kworker/0:1 Not tainted 5.5.0-rc1-ste-00003-gb4be21f316ac-dirty #56
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS ?-20190727_073836-buildvm-ppc64le-16.ppc.fedoraproject.org-3.fc31 04/01/2014
Workqueue: virtio_vsock virtio_transport_rx_work [vmw_vsock_virtio_transport]
RIP: 0010:virtio_transport_send_pkt_info+0x20/0x130 [vmw_vsock_virtio_transport_common]
Code: 1f 84 00 00 00 00 00 0f 1f 00 55 48 89 e5 41 57 41 56 41 55 49 89 f5 41 54 49 89 fc 53 48 83 ec 10 44 8b 76 20 e8 c0 ba fe ff <48> 8b 80 e8 00 00 00 e8 64 e3 7d c1 45 8b 45 00 41 8b 8c 24 d4 02
RSP: 0018:ffffc900000b7d08 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff88807bf12728 RCX: 0000000000000000
RDX: ffff88807bf12700 RSI: ffffc900000b7d50 RDI: ffff888035c84000
RBP: ffffc900000b7d40 R08: ffff888035c84000 R09: ffffc900000b7d08
R10: ffff8880781de800 R11: 0000000000000018 R12: ffff888035c84000
R13: ffffc900000b7d50 R14: 0000000000000000 R15: ffff88807bf12724
FS: 0000000000000000(0000) GS:ffff88807dc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000000000e8 CR3: 00000000790f4004 CR4: 0000000000160ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
virtio_transport_reset+0x59/0x70 [vmw_vsock_virtio_transport_common]
virtio_transport_recv_pkt+0x5bb/0xe50 [vmw_vsock_virtio_transport_common]
? detach_buf_split+0xf1/0x130
virtio_transport_rx_work+0xba/0x130 [vmw_vsock_virtio_transport]
process_one_work+0x1c0/0x300
worker_thread+0x45/0x3c0
kthread+0xfc/0x130
? current_work+0x40/0x40
? kthread_park+0x90/0x90
ret_from_fork+0x35/0x40
Modules linked in: sunrpc kvm_intel kvm vmw_vsock_virtio_transport vmw_vsock_virtio_transport_common irqbypass vsock virtio_rng rng_core
CR2: 00000000000000e8
---[ end trace e75400e2ea2fa824 ]---
This happens because virtio_transport_reset() calls
virtio_transport_send_pkt_info() that can be used only on
connecting/connected sockets.
This patch fixes the issue, using virtio_transport_reset_no_sock()
instead of virtio_transport_reset() when we are handling a listener
socket.
Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport_common.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index e5ea29c6bca7..f5991006190e 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1021,18 +1021,18 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt,
int ret;
if (le16_to_cpu(pkt->hdr.op) != VIRTIO_VSOCK_OP_REQUEST) {
- virtio_transport_reset(vsk, pkt);
+ virtio_transport_reset_no_sock(t, pkt);
return -EINVAL;
}
if (sk_acceptq_is_full(sk)) {
- virtio_transport_reset(vsk, pkt);
+ virtio_transport_reset_no_sock(t, pkt);
return -ENOMEM;
}
child = vsock_create_connected(sk);
if (!child) {
- virtio_transport_reset(vsk, pkt);
+ virtio_transport_reset_no_sock(t, pkt);
return -ENOMEM;
}
@@ -1054,7 +1054,7 @@ virtio_transport_recv_listen(struct sock *sk, struct virtio_vsock_pkt *pkt,
*/
if (ret || vchild->transport != &t->transport) {
release_sock(child);
- virtio_transport_reset(vsk, pkt);
+ virtio_transport_reset_no_sock(t, pkt);
sock_put(child);
return ret;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH net 2/2] vsock/virtio: add WARN_ON check on virtio_transport_get_ops()
2019-12-13 18:47 [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions Stefano Garzarella
2019-12-13 18:48 ` [PATCH net 1/2] vsock/virtio: fix null-pointer dereference in virtio_transport_recv_listen() Stefano Garzarella
@ 2019-12-13 18:48 ` Stefano Garzarella
2019-12-17 0:07 ` [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Stefano Garzarella @ 2019-12-13 18:48 UTC (permalink / raw)
To: davem
Cc: Stefano Garzarella, linux-kernel, netdev, kvm, virtualization,
Stefan Hajnoczi
virtio_transport_get_ops() and virtio_transport_send_pkt_info()
can only be used on connecting/connected sockets, since a socket
assigned to a transport is required.
This patch adds a WARN_ON() on virtio_transport_get_ops() to check
this requirement, a comment and a returned error on
virtio_transport_send_pkt_info(),
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport_common.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index f5991006190e..6abec3fc81d1 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -34,6 +34,9 @@ virtio_transport_get_ops(struct vsock_sock *vsk)
{
const struct vsock_transport *t = vsock_core_get_transport(vsk);
+ if (WARN_ON(!t))
+ return NULL;
+
return container_of(t, struct virtio_transport, transport);
}
@@ -161,15 +164,25 @@ void virtio_transport_deliver_tap_pkt(struct virtio_vsock_pkt *pkt)
}
EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt);
+/* This function can only be used on connecting/connected sockets,
+ * since a socket assigned to a transport is required.
+ *
+ * Do not use on listener sockets!
+ */
static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
struct virtio_vsock_pkt_info *info)
{
u32 src_cid, src_port, dst_cid, dst_port;
+ const struct virtio_transport *t_ops;
struct virtio_vsock_sock *vvs;
struct virtio_vsock_pkt *pkt;
u32 pkt_len = info->pkt_len;
- src_cid = virtio_transport_get_ops(vsk)->transport.get_local_cid();
+ t_ops = virtio_transport_get_ops(vsk);
+ if (unlikely(!t_ops))
+ return -EFAULT;
+
+ src_cid = t_ops->transport.get_local_cid();
src_port = vsk->local_addr.svm_port;
if (!info->remote_cid) {
dst_cid = vsk->remote_addr.svm_cid;
@@ -202,7 +215,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
virtio_transport_inc_tx_pkt(vvs, pkt);
- return virtio_transport_get_ops(vsk)->send_pkt(pkt);
+ return t_ops->send_pkt(pkt);
}
static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
--
2.23.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions
2019-12-13 18:47 [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions Stefano Garzarella
2019-12-13 18:48 ` [PATCH net 1/2] vsock/virtio: fix null-pointer dereference in virtio_transport_recv_listen() Stefano Garzarella
2019-12-13 18:48 ` [PATCH net 2/2] vsock/virtio: add WARN_ON check on virtio_transport_get_ops() Stefano Garzarella
@ 2019-12-17 0:07 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2019-12-17 0:07 UTC (permalink / raw)
To: sgarzare; +Cc: linux-kernel, netdev, kvm, virtualization, stefanha
From: Stefano Garzarella <sgarzare@redhat.com>
Date: Fri, 13 Dec 2019 19:47:59 +0100
> This series mainly solves a possible null-pointer dereference in
> virtio_transport_recv_listen() introduced with the multi-transport
> support [PATCH 1].
>
> PATCH 2 adds a WARN_ON check for the same potential issue
> and a returned error in the virtio_transport_send_pkt_info() function
> to avoid crashing the kernel.
Series applied, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-17 0:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-13 18:47 [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions Stefano Garzarella
2019-12-13 18:48 ` [PATCH net 1/2] vsock/virtio: fix null-pointer dereference in virtio_transport_recv_listen() Stefano Garzarella
2019-12-13 18:48 ` [PATCH net 2/2] vsock/virtio: add WARN_ON check on virtio_transport_get_ops() Stefano Garzarella
2019-12-17 0:07 ` [PATCH net 0/2] vsock/virtio: fix null-pointer dereference and related precautions David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).