search for: virtio_transport_reset_no_sock

Previous to commit 22b5c0b63f32 ("vsock/virtio: fix kernel panic after device hot-unplug"), vsock_core_init() was called from virtio_vsock_probe(). Now, virtio_transport_reset_no_sock() can be called before vsock_core_init() has the chance to run. [Wed Feb 27 14:17:09 2019] BUG: unable to handle kernel NULL pointer dereference at 0000000000000110 [Wed Feb 27 14:17:09 2019] #PF error: [normal kernel read fault] [Wed Feb 27 14:17:09 2019] PGD 0 P4D 0 [Wed Feb 27 14:17:09 2019] Oo...

Previous to commit 22b5c0b63f32 ("vsock/virtio: fix kernel panic after device hot-unplug"), vsock_core_init() was called from virtio_vsock_probe(). Now, virtio_transport_reset_no_sock() can be called before vsock_core_init() has the chance to run. [Wed Feb 27 14:17:09 2019] BUG: unable to handle kernel NULL pointer dereference at 0000000000000110 [Wed Feb 27 14:17:09 2019] #PF error: [normal kernel read fault] [Wed Feb 27 14:17:09 2019] PGD 0 P4D 0 [Wed Feb 27 14:17:09 2019] Oo...

We are going to add 'struct vsock_sock *' parameter to virtio_transport_get_ops(). In some cases, like in the virtio_transport_reset_no_sock(), we don't have any socket assigned to the packet received, so we can't use the virtio_transport_get_ops(). In order to allow virtio_transport_reset_no_sock() to use the '.send_pkt' callback from the 'vhost_transport' or 'virtio_transport', we add the 'struct v...

...thanks for catching this issue, I have a comment below. On Tue, Mar 05, 2019 at 08:01:45PM +0200, Adalbert Laz?r wrote: > Previous to commit 22b5c0b63f32 ("vsock/virtio: fix kernel panic after device hot-unplug"), > vsock_core_init() was called from virtio_vsock_probe(). Now, > virtio_transport_reset_no_sock() can be called before vsock_core_init() > has the chance to run. > > [Wed Feb 27 14:17:09 2019] BUG: unable to handle kernel NULL pointer dereference at 0000000000000110 > [Wed Feb 27 14:17:09 2019] #PF error: [normal kernel read fault] > [Wed Feb 27 14:17:09 2019] PGD 0 P4D 0 >...

On Tue, Mar 05, 2019 at 08:01:45PM +0200, Adalbert Laz?r wrote: Thanks for the patch, Adalbert! Please add a Signed-off-by tag so your patch can be merged (see Documentation/process/submitting-patches.rst Chapter 11 for details on the Developer's Certificate of Origin). > static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *pkt) > { > + const struct virtio_transport *t; > struct virtio_vsock_pkt_info info = { > .op = VIRTIO_VSOCK_OP_RST, > .type = le16_to_cpu(pkt->hdr.type), > @@ -680,7 +681,11 @@ static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *...

Previous to commit 22b5c0b63f32 ("vsock/virtio: fix kernel panic after device hot-unplug"), vsock_core_init() was called from virtio_vsock_probe(). Now, virtio_transport_reset_no_sock() can be called before vsock_core_init() has the chance to run. [Wed Feb 27 14:17:09 2019] BUG: unable to handle kernel NULL pointer dereference at 0000000000000110 [Wed Feb 27 14:17:09 2019] #PF error: [normal kernel read fault] [Wed Feb 27 14:17:09 2019] PGD 0 P4D 0 [Wed Feb 27 14:17:09 2019] Oo...

Previous to commit 22b5c0b63f32 ("vsock/virtio: fix kernel panic after device hot-unplug"), vsock_core_init() was called from virtio_vsock_probe(). Now, virtio_transport_reset_no_sock() can be called before vsock_core_init() has the chance to run. [Wed Feb 27 14:17:09 2019] BUG: unable to handle kernel NULL pointer dereference at 0000000000000110 [Wed Feb 27 14:17:09 2019] #PF error: [normal kernel read fault] [Wed Feb 27 14:17:09 2019] PGD 0 P4D 0 [Wed Feb 27 14:17:09 2019] Oo...

On Wed, Mar 06, 2019 at 12:13:53PM +0200, Adalbert Laz?r wrote: > Previous to commit 22b5c0b63f32 ("vsock/virtio: fix kernel panic > after device hot-unplug"), vsock_core_init() was called from > virtio_vsock_probe(). Now, virtio_transport_reset_no_sock() can be called > before vsock_core_init() has the chance to run. > > [Wed Feb 27 14:17:09 2019] BUG: unable to handle kernel NULL pointer dereference at 0000000000000110 > [Wed Feb 27 14:17:09 2019] #PF error: [normal kernel read fault] > [Wed Feb 27 14:17:09 2019] PGD 0 P4D 0 >...

...8:01:45PM +0200, Adalbert Laz?r wrote: > > Thanks for the patch, Adalbert! Please add a Signed-off-by tag so your > patch can be merged (see Documentation/process/submitting-patches.rst > Chapter 11 for details on the Developer's Certificate of Origin). > > > static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *pkt) > > { > > + const struct virtio_transport *t; > > struct virtio_vsock_pkt_info info = { > > .op = VIRTIO_VSOCK_OP_RST, > > .type = le16_to_cpu(pkt->hdr.type), > > @@ -680,7 +681,11 @@ static int virtio_transport_reset_no_...

...8:01:45PM +0200, Adalbert Laz?r wrote: > > Thanks for the patch, Adalbert! Please add a Signed-off-by tag so your > patch can be merged (see Documentation/process/submitting-patches.rst > Chapter 11 for details on the Developer's Certificate of Origin). > > > static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *pkt) > > { > > + const struct virtio_transport *t; > > struct virtio_vsock_pkt_info info = { > > .op = VIRTIO_VSOCK_OP_RST, > > .type = le16_to_cpu(pkt->hdr.type), > > @@ -680,7 +681,11 @@ static int virtio_transport_reset_no_...

On Wed, Mar 06, 2019 at 11:10:41AM +0200, Adalbert Laz?r wrote: > On Wed, 6 Mar 2019 08:41:04 +0000, Stefan Hajnoczi <stefanha at gmail.com> wrote: > > On Tue, Mar 05, 2019 at 08:01:45PM +0200, Adalbert Laz?r wrote: > > The pkt argument is the received packet that we must reply to. > > The reply packet is allocated just before line 680 and must be free > >

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

...ns(+), 7 deletions(-) > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index 6120384..22e99c4 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -606,9 +606,9 @@ static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *pkt) > return 0; > > pkt = virtio_transport_alloc_pkt(&info, 0, > - le32_to_cpu(pkt->hdr.dst_cid), > + le64_to_cpu(pkt->hdr.dst_cid), > le32_to_cpu(pkt->hdr.dst_port), > - le32_to_cpu(pkt->hdr.src_cid), &...

...ns(+), 7 deletions(-) > > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c > index 6120384..22e99c4 100644 > --- a/net/vmw_vsock/virtio_transport_common.c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -606,9 +606,9 @@ static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *pkt) > return 0; > > pkt = virtio_transport_alloc_pkt(&info, 0, > - le32_to_cpu(pkt->hdr.dst_cid), > + le64_to_cpu(pkt->hdr.dst_cid), > le32_to_cpu(pkt->hdr.dst_port), > - le32_to_cpu(pkt->hdr.src_cid), &...

...Stefano Stefano Garzarella (13): vsock/vmci: remove unused VSOCK_DEFAULT_CONNECT_TIMEOUT vsock: remove vm_sockets_get_local_cid() vsock: remove include/linux/vm_sockets.h file vsock: add 'transport' member in the struct vsock_sock vsock/virtio: add transport parameter to the virtio_transport_reset_no_sock() vsock: add 'struct vsock_sock *' param to vsock_core_get_transport() vsock: handle buffer_size sockopts in the core vsock: move vsock_insert_unbound() in the vsock_create() hv_sock: set VMADDR_CID_HOST in the hvs_remote_addr_init() vsock: add multi-transports support vsock: ad...

...Stefano Stefano Garzarella (13): vsock/vmci: remove unused VSOCK_DEFAULT_CONNECT_TIMEOUT vsock: remove vm_sockets_get_local_cid() vsock: remove include/linux/vm_sockets.h file vsock: add 'transport' member in the struct vsock_sock vsock/virtio: add transport parameter to the virtio_transport_reset_no_sock() vsock: add 'struct vsock_sock *' param to vsock_core_get_transport() vsock: handle buffer_size sockopts in the core vsock: move vsock_insert_unbound() in the vsock_create() hv_sock: set VMADDR_CID_HOST in the hvs_remote_addr_init() vsock: add multi-transports support vsock: ad...

...x guest. Stefano Garzarella (15): vsock/vmci: remove unused VSOCK_DEFAULT_CONNECT_TIMEOUT vsock: remove vm_sockets_get_local_cid() vsock: remove include/linux/vm_sockets.h file vsock: add 'transport' member in the struct vsock_sock vsock/virtio: add transport parameter to the virtio_transport_reset_no_sock() vsock: add 'struct vsock_sock *' param to vsock_core_get_transport() vsock: handle buffer_size sockopts in the core vsock: add vsock_create_connected() called by transports vsock: move vsock_insert_unbound() in the vsock_create() hv_sock: set VMADDR_CID_HOST in the hvs_remote_ad...

...- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 6120384..22e99c4 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -606,9 +606,9 @@ static int virtio_transport_reset_no_sock(struct virtio_vsock_pkt *pkt) return 0; pkt = virtio_transport_alloc_pkt(&info, 0, - le32_to_cpu(pkt->hdr.dst_cid), + le64_to_cpu(pkt->hdr.dst_cid), le32_to_cpu(pkt->hdr.dst_port), - le32_to_cpu(pkt->hdr.src_cid), + le64_to_cpu(pkt->hdr.src_cid),...

...*t, > > lock_sock(sk); > > + /* Check it again if vsk is removed by vsock_remove_sock */ > + spin_lock_bh(&vsock_table_lock); > + if (!__vsock_in_bound_table(vsk) && !__vsock_in_connected_table(vsk)) { > + spin_unlock_bh(&vsock_table_lock); > + (void)virtio_transport_reset_no_sock(t, pkt); > + release_sock(sk); > + sock_put(sk); > + goto free_pkt; > + } > + spin_unlock_bh(&vsock_table_lock); > + As an a simpler alternative, can we check the sk_shutdown or the socket state without check again both bound and connected tables? This is a data path, so...

...c > +++ b/net/vmw_vsock/virtio_transport_common.c > @@ -1132,6 +1132,14 @@ void virtio_transport_recv_pkt(struct virtio_transport *t, > > lock_sock(sk); > > + /* Check if sk has been released before lock_sock */ > + if (sk->sk_shutdown == SHUTDOWN_MASK) { > + (void)virtio_transport_reset_no_sock(t, pkt); > + release_sock(sk); > + sock_put(sk); > + goto free_pkt; > + } > + > /* Update CID in case it has changed after a transport reset event */ > vsk->local_addr.svm_cid = dst.svm_cid; > > -- > 2.17.1 > Anyway, the patch LGTM, let see what David...