* [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt
@ 2020-05-29 15:21 Jia He
2020-05-29 16:34 ` Stefano Garzarella
2020-06-05 14:10 ` Sasha Levin
0 siblings, 2 replies; 7+ messages in thread
From: Jia He @ 2020-05-29 15:21 UTC (permalink / raw)
To: Stefan Hajnoczi, Stefano Garzarella
Cc: David S. Miller, Jakub Kicinski, kvm, virtualization, netdev,
linux-kernel, Kaly Xin, Jia He, stable
When client tries to connect(SOCK_STREAM) the server in the guest with
NONBLOCK mode, there will be a panic on a ThunderX2 (armv8a server):
[ 463.718844][ T5040] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[ 463.718848][ T5040] Mem abort info:
[ 463.718849][ T5040] ESR = 0x96000044
[ 463.718852][ T5040] EC = 0x25: DABT (current EL), IL = 32 bits
[ 463.718853][ T5040] SET = 0, FnV = 0
[ 463.718854][ T5040] EA = 0, S1PTW = 0
[ 463.718855][ T5040] Data abort info:
[ 463.718856][ T5040] ISV = 0, ISS = 0x00000044
[ 463.718857][ T5040] CM = 0, WnR = 1
[ 463.718859][ T5040] user pgtable: 4k pages, 48-bit VAs, pgdp=0000008f6f6e9000
[ 463.718861][ T5040] [0000000000000000] pgd=0000000000000000
[ 463.718866][ T5040] Internal error: Oops: 96000044 [#1] SMP
[...]
[ 463.718977][ T5040] CPU: 213 PID: 5040 Comm: vhost-5032 Tainted: G O 5.7.0-rc7+ #139
[ 463.718980][ T5040] Hardware name: GIGABYTE R281-T91-00/MT91-FS1-00, BIOS F06 09/25/2018
[ 463.718982][ T5040] pstate: 60400009 (nZCv daif +PAN -UAO)
[ 463.718995][ T5040] pc : virtio_transport_recv_pkt+0x4c8/0xd40 [vmw_vsock_virtio_transport_common]
[ 463.718999][ T5040] lr : virtio_transport_recv_pkt+0x1fc/0xd40 [vmw_vsock_virtio_transport_common]
[ 463.719000][ T5040] sp : ffff80002dbe3c40
[...]
[ 463.719025][ T5040] Call trace:
[ 463.719030][ T5040] virtio_transport_recv_pkt+0x4c8/0xd40 [vmw_vsock_virtio_transport_common]
[ 463.719034][ T5040] vhost_vsock_handle_tx_kick+0x360/0x408 [vhost_vsock]
[ 463.719041][ T5040] vhost_worker+0x100/0x1a0 [vhost]
[ 463.719048][ T5040] kthread+0x128/0x130
[ 463.719052][ T5040] ret_from_fork+0x10/0x18
The race condition as follows:
Task1 Task2
===== =====
__sock_release virtio_transport_recv_pkt
__vsock_release vsock_find_bound_socket (found)
lock_sock_nested
vsock_remove_sock
sock_orphan
sk_set_socket(sk, NULL)
...
release_sock
lock_sock
virtio_transport_recv_connecting
sk->sk_socket->state (panic)
The root cause is that vsock_find_bound_socket can't hold the lock_sock,
so there is a small race window between vsock_find_bound_socket() and
lock_sock(). If there is __vsock_release() in another task, sk->sk_socket
will be set to NULL inadvertently.
This fixes it by checking sk->sk_shutdown.
Signed-off-by: Jia He <justin.he@arm.com>
Cc: stable@vger.kernel.org
Cc: Stefano Garzarella <sgarzare@redhat.com>
---
v2: use lightweight checking suggested by Stefano Garzarella
net/vmw_vsock/virtio_transport_common.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 69efc891885f..0edda1edf988 100644
--- a/net/vmw_vsock/virtio_transport_common.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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt
2020-05-29 15:21 [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt Jia He
@ 2020-05-29 16:34 ` Stefano Garzarella
2020-05-30 1:14 ` Justin He
2020-06-05 14:10 ` Sasha Levin
1 sibling, 1 reply; 7+ messages in thread
From: Stefano Garzarella @ 2020-05-29 16:34 UTC (permalink / raw)
To: Jia He
Cc: Stefan Hajnoczi, David S. Miller, Jakub Kicinski, kvm,
virtualization, netdev, linux-kernel, Kaly Xin, stable
On Fri, May 29, 2020 at 11:21:02PM +0800, Jia He wrote:
> When client tries to connect(SOCK_STREAM) the server in the guest with
> NONBLOCK mode, there will be a panic on a ThunderX2 (armv8a server):
> [ 463.718844][ T5040] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [ 463.718848][ T5040] Mem abort info:
> [ 463.718849][ T5040] ESR = 0x96000044
> [ 463.718852][ T5040] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 463.718853][ T5040] SET = 0, FnV = 0
> [ 463.718854][ T5040] EA = 0, S1PTW = 0
> [ 463.718855][ T5040] Data abort info:
> [ 463.718856][ T5040] ISV = 0, ISS = 0x00000044
> [ 463.718857][ T5040] CM = 0, WnR = 1
> [ 463.718859][ T5040] user pgtable: 4k pages, 48-bit VAs, pgdp=0000008f6f6e9000
> [ 463.718861][ T5040] [0000000000000000] pgd=0000000000000000
> [ 463.718866][ T5040] Internal error: Oops: 96000044 [#1] SMP
> [...]
> [ 463.718977][ T5040] CPU: 213 PID: 5040 Comm: vhost-5032 Tainted: G O 5.7.0-rc7+ #139
> [ 463.718980][ T5040] Hardware name: GIGABYTE R281-T91-00/MT91-FS1-00, BIOS F06 09/25/2018
> [ 463.718982][ T5040] pstate: 60400009 (nZCv daif +PAN -UAO)
> [ 463.718995][ T5040] pc : virtio_transport_recv_pkt+0x4c8/0xd40 [vmw_vsock_virtio_transport_common]
> [ 463.718999][ T5040] lr : virtio_transport_recv_pkt+0x1fc/0xd40 [vmw_vsock_virtio_transport_common]
> [ 463.719000][ T5040] sp : ffff80002dbe3c40
> [...]
> [ 463.719025][ T5040] Call trace:
> [ 463.719030][ T5040] virtio_transport_recv_pkt+0x4c8/0xd40 [vmw_vsock_virtio_transport_common]
> [ 463.719034][ T5040] vhost_vsock_handle_tx_kick+0x360/0x408 [vhost_vsock]
> [ 463.719041][ T5040] vhost_worker+0x100/0x1a0 [vhost]
> [ 463.719048][ T5040] kthread+0x128/0x130
> [ 463.719052][ T5040] ret_from_fork+0x10/0x18
^ ^
Maybe we can remove these two columns from the commit message.
>
> The race condition as follows:
> Task1 Task2
> ===== =====
> __sock_release virtio_transport_recv_pkt
> __vsock_release vsock_find_bound_socket (found)
> lock_sock_nested
> vsock_remove_sock
> sock_orphan
> sk_set_socket(sk, NULL)
Here we can add:
sk->sk_shutdown = SHUTDOWN_MASK;
> ...
> release_sock
> lock_sock
> virtio_transport_recv_connecting
> sk->sk_socket->state (panic)
>
> The root cause is that vsock_find_bound_socket can't hold the lock_sock,
> so there is a small race window between vsock_find_bound_socket() and
> lock_sock(). If there is __vsock_release() in another task, sk->sk_socket
> will be set to NULL inadvertently.
>
> This fixes it by checking sk->sk_shutdown.
>
> Signed-off-by: Jia He <justin.he@arm.com>
> Cc: stable@vger.kernel.org
> Cc: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v2: use lightweight checking suggested by Stefano Garzarella
>
> net/vmw_vsock/virtio_transport_common.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 69efc891885f..0edda1edf988 100644
> --- a/net/vmw_vsock/virtio_transport_common.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 and other say.
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Thanks,
Stefano
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt
2020-05-29 16:34 ` Stefano Garzarella
@ 2020-05-30 1:14 ` Justin He
0 siblings, 0 replies; 7+ messages in thread
From: Justin He @ 2020-05-30 1:14 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Stefan Hajnoczi, David S. Miller, Jakub Kicinski, kvm,
virtualization, netdev, linux-kernel, Kaly Xin, stable
Hi Stefano
> -----Original Message-----
> From: Stefano Garzarella <sgarzare@redhat.com>
> Sent: Saturday, May 30, 2020 12:34 AM
> To: Justin He <Justin.He@arm.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>; David S. Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>;
> kvm@vger.kernel.org; virtualization@lists.linux-foundation.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Kaly Xin
> <Kaly.Xin@arm.com>; stable@vger.kernel.org
> Subject: Re: [PATCH v2] virtio_vsock: Fix race condition in
> virtio_transport_recv_pkt
>
> On Fri, May 29, 2020 at 11:21:02PM +0800, Jia He wrote:
> > When client tries to connect(SOCK_STREAM) the server in the guest with
> > NONBLOCK mode, there will be a panic on a ThunderX2 (armv8a server):
> > [ 463.718844][ T5040] Unable to handle kernel NULL pointer dereference
> at virtual address 0000000000000000
> > [ 463.718848][ T5040] Mem abort info:
> > [ 463.718849][ T5040] ESR = 0x96000044
> > [ 463.718852][ T5040] EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 463.718853][ T5040] SET = 0, FnV = 0
> > [ 463.718854][ T5040] EA = 0, S1PTW = 0
> > [ 463.718855][ T5040] Data abort info:
> > [ 463.718856][ T5040] ISV = 0, ISS = 0x00000044
> > [ 463.718857][ T5040] CM = 0, WnR = 1
> > [ 463.718859][ T5040] user pgtable: 4k pages, 48-bit VAs,
> pgdp=0000008f6f6e9000
> > [ 463.718861][ T5040] [0000000000000000] pgd=0000000000000000
> > [ 463.718866][ T5040] Internal error: Oops: 96000044 [#1] SMP
> > [...]
> > [ 463.718977][ T5040] CPU: 213 PID: 5040 Comm: vhost-5032 Tainted: G
> O 5.7.0-rc7+ #139
> > [ 463.718980][ T5040] Hardware name: GIGABYTE R281-T91-00/MT91-FS1-00,
> BIOS F06 09/25/2018
> > [ 463.718982][ T5040] pstate: 60400009 (nZCv daif +PAN -UAO)
> > [ 463.718995][ T5040] pc : virtio_transport_recv_pkt+0x4c8/0xd40
> [vmw_vsock_virtio_transport_common]
> > [ 463.718999][ T5040] lr : virtio_transport_recv_pkt+0x1fc/0xd40
> [vmw_vsock_virtio_transport_common]
> > [ 463.719000][ T5040] sp : ffff80002dbe3c40
> > [...]
> > [ 463.719025][ T5040] Call trace:
> > [ 463.719030][ T5040] virtio_transport_recv_pkt+0x4c8/0xd40
> [vmw_vsock_virtio_transport_common]
> > [ 463.719034][ T5040] vhost_vsock_handle_tx_kick+0x360/0x408
> [vhost_vsock]
> > [ 463.719041][ T5040] vhost_worker+0x100/0x1a0 [vhost]
> > [ 463.719048][ T5040] kthread+0x128/0x130
> > [ 463.719052][ T5040] ret_from_fork+0x10/0x18
> ^ ^
> Maybe we can remove these two columns from the commit message.
>
> >
> > The race condition as follows:
> > Task1 Task2
> > ===== =====
> > __sock_release virtio_transport_recv_pkt
> > __vsock_release vsock_find_bound_socket (found)
> > lock_sock_nested
> > vsock_remove_sock
> > sock_orphan
> > sk_set_socket(sk, NULL)
>
> Here we can add:
> sk->sk_shutdown = SHUTDOWN_MASK;
Indeed. This makes it more clearly
--
Cheers,
Justin (Jia He)
>
> > ...
> > release_sock
> > lock_sock
> > virtio_transport_recv_connecting
> > sk->sk_socket->state (panic)
> >
> > The root cause is that vsock_find_bound_socket can't hold the lock_sock,
> > so there is a small race window between vsock_find_bound_socket() and
> > lock_sock(). If there is __vsock_release() in another task, sk->sk_socket
> > will be set to NULL inadvertently.
> >
> > This fixes it by checking sk->sk_shutdown.
> >
> > Signed-off-by: Jia He <justin.he@arm.com>
> > Cc: stable@vger.kernel.org
> > Cc: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> > v2: use lightweight checking suggested by Stefano Garzarella
> >
> > net/vmw_vsock/virtio_transport_common.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> > index 69efc891885f..0edda1edf988 100644
> > --- a/net/vmw_vsock/virtio_transport_common.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 and other say.
>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
> Thanks,
> Stefano
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt
@ 2020-05-30 1:14 ` Justin He
0 siblings, 0 replies; 7+ messages in thread
From: Justin He @ 2020-05-30 1:14 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Stefan Hajnoczi, David S. Miller, Jakub Kicinski, kvm,
virtualization, netdev, linux-kernel, Kaly Xin, stable
Hi Stefano
> -----Original Message-----
> From: Stefano Garzarella <sgarzare@redhat.com>
> Sent: Saturday, May 30, 2020 12:34 AM
> To: Justin He <Justin.He@arm.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>; David S. Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>;
> kvm@vger.kernel.org; virtualization@lists.linux-foundation.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Kaly Xin
> <Kaly.Xin@arm.com>; stable@vger.kernel.org
> Subject: Re: [PATCH v2] virtio_vsock: Fix race condition in
> virtio_transport_recv_pkt
>
> On Fri, May 29, 2020 at 11:21:02PM +0800, Jia He wrote:
> > When client tries to connect(SOCK_STREAM) the server in the guest with
> > NONBLOCK mode, there will be a panic on a ThunderX2 (armv8a server):
> > [ 463.718844][ T5040] Unable to handle kernel NULL pointer dereference
> at virtual address 0000000000000000
> > [ 463.718848][ T5040] Mem abort info:
> > [ 463.718849][ T5040] ESR = 0x96000044
> > [ 463.718852][ T5040] EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 463.718853][ T5040] SET = 0, FnV = 0
> > [ 463.718854][ T5040] EA = 0, S1PTW = 0
> > [ 463.718855][ T5040] Data abort info:
> > [ 463.718856][ T5040] ISV = 0, ISS = 0x00000044
> > [ 463.718857][ T5040] CM = 0, WnR = 1
> > [ 463.718859][ T5040] user pgtable: 4k pages, 48-bit VAs,
> pgdp=0000008f6f6e9000
> > [ 463.718861][ T5040] [0000000000000000] pgd=0000000000000000
> > [ 463.718866][ T5040] Internal error: Oops: 96000044 [#1] SMP
> > [...]
> > [ 463.718977][ T5040] CPU: 213 PID: 5040 Comm: vhost-5032 Tainted: G
> O 5.7.0-rc7+ #139
> > [ 463.718980][ T5040] Hardware name: GIGABYTE R281-T91-00/MT91-FS1-00,
> BIOS F06 09/25/2018
> > [ 463.718982][ T5040] pstate: 60400009 (nZCv daif +PAN -UAO)
> > [ 463.718995][ T5040] pc : virtio_transport_recv_pkt+0x4c8/0xd40
> [vmw_vsock_virtio_transport_common]
> > [ 463.718999][ T5040] lr : virtio_transport_recv_pkt+0x1fc/0xd40
> [vmw_vsock_virtio_transport_common]
> > [ 463.719000][ T5040] sp : ffff80002dbe3c40
> > [...]
> > [ 463.719025][ T5040] Call trace:
> > [ 463.719030][ T5040] virtio_transport_recv_pkt+0x4c8/0xd40
> [vmw_vsock_virtio_transport_common]
> > [ 463.719034][ T5040] vhost_vsock_handle_tx_kick+0x360/0x408
> [vhost_vsock]
> > [ 463.719041][ T5040] vhost_worker+0x100/0x1a0 [vhost]
> > [ 463.719048][ T5040] kthread+0x128/0x130
> > [ 463.719052][ T5040] ret_from_fork+0x10/0x18
> ^ ^
> Maybe we can remove these two columns from the commit message.
>
> >
> > The race condition as follows:
> > Task1 Task2
> > ===== =====
> > __sock_release virtio_transport_recv_pkt
> > __vsock_release vsock_find_bound_socket (found)
> > lock_sock_nested
> > vsock_remove_sock
> > sock_orphan
> > sk_set_socket(sk, NULL)
>
> Here we can add:
> sk->sk_shutdown = SHUTDOWN_MASK;
Indeed. This makes it more clearly
--
Cheers,
Justin (Jia He)
>
> > ...
> > release_sock
> > lock_sock
> > virtio_transport_recv_connecting
> > sk->sk_socket->state (panic)
> >
> > The root cause is that vsock_find_bound_socket can't hold the lock_sock,
> > so there is a small race window between vsock_find_bound_socket() and
> > lock_sock(). If there is __vsock_release() in another task, sk->sk_socket
> > will be set to NULL inadvertently.
> >
> > This fixes it by checking sk->sk_shutdown.
> >
> > Signed-off-by: Jia He <justin.he@arm.com>
> > Cc: stable@vger.kernel.org
> > Cc: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> > v2: use lightweight checking suggested by Stefano Garzarella
> >
> > net/vmw_vsock/virtio_transport_common.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> > index 69efc891885f..0edda1edf988 100644
> > --- a/net/vmw_vsock/virtio_transport_common.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 and other say.
>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
> Thanks,
> Stefano
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt
2020-05-29 15:21 [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt Jia He
2020-05-29 16:34 ` Stefano Garzarella
@ 2020-06-05 14:10 ` Sasha Levin
2020-06-05 14:29 ` Stefano Garzarella
1 sibling, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2020-06-05 14:10 UTC (permalink / raw)
To: Sasha Levin, Jia He, Stefan Hajnoczi
Cc: David S. Miller, stable, Stefano Garzarella, stable
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182, v4.9.225, v4.4.225.
v5.6.15: Build OK!
v5.4.43: Build failed! Errors:
net/vmw_vsock/virtio_transport_common.c:1109:40: error: ‘t’ undeclared (first use in this function); did you mean ‘tm’?
net/vmw_vsock/virtio_transport_common.c:1109:9: error: too many arguments to function ‘virtio_transport_reset_no_sock’
v4.19.125: Build failed! Errors:
net/vmw_vsock/virtio_transport_common.c:1042:40: error: ‘t’ undeclared (first use in this function); did you mean ‘tm’?
net/vmw_vsock/virtio_transport_common.c:1042:9: error: too many arguments to function ‘virtio_transport_reset_no_sock’
v4.14.182: Build failed! Errors:
net/vmw_vsock/virtio_transport_common.c:1038:40: error: ‘t’ undeclared (first use in this function); did you mean ‘tm’?
net/vmw_vsock/virtio_transport_common.c:1038:9: error: too many arguments to function ‘virtio_transport_reset_no_sock’
v4.9.225: Build failed! Errors:
net/vmw_vsock/virtio_transport_common.c:968:40: error: ‘t’ undeclared (first use in this function); did you mean ‘tm’?
net/vmw_vsock/virtio_transport_common.c:968:9: error: too many arguments to function ‘virtio_transport_reset_no_sock’
v4.4.225: Failed to apply! Possible dependencies:
06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt
2020-06-05 14:10 ` Sasha Levin
@ 2020-06-05 14:29 ` Stefano Garzarella
2020-06-08 3:06 ` Justin He
0 siblings, 1 reply; 7+ messages in thread
From: Stefano Garzarella @ 2020-06-05 14:29 UTC (permalink / raw)
To: Sasha Levin; +Cc: Jia He, Stefan Hajnoczi, David S. Miller, stable
On Fri, Jun 05, 2020 at 02:10:49PM +0000, Sasha Levin wrote:
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125, v4.14.182, v4.9.225, v4.4.225.
>
> v5.6.15: Build OK!
> v5.4.43: Build failed! Errors:
> net/vmw_vsock/virtio_transport_common.c:1109:40: error: ???????????????t?????????????????? undeclared (first use in this function); did you mean ???????????????tm???????????????????
> net/vmw_vsock/virtio_transport_common.c:1109:9: error: too many arguments to function ???????????????virtio_transport_reset_no_sock??????????????????
>
> v4.19.125: Build failed! Errors:
> net/vmw_vsock/virtio_transport_common.c:1042:40: error: ???????????????t?????????????????? undeclared (first use in this function); did you mean ???????????????tm???????????????????
> net/vmw_vsock/virtio_transport_common.c:1042:9: error: too many arguments to function ???????????????virtio_transport_reset_no_sock??????????????????
>
> v4.14.182: Build failed! Errors:
> net/vmw_vsock/virtio_transport_common.c:1038:40: error: ???????????????t?????????????????? undeclared (first use in this function); did you mean ???????????????tm???????????????????
> net/vmw_vsock/virtio_transport_common.c:1038:9: error: too many arguments to function ???????????????virtio_transport_reset_no_sock??????????????????
>
> v4.9.225: Build failed! Errors:
> net/vmw_vsock/virtio_transport_common.c:968:40: error: ???????????????t?????????????????? undeclared (first use in this function); did you mean ???????????????tm???????????????????
> net/vmw_vsock/virtio_transport_common.c:968:9: error: too many arguments to function ???????????????virtio_transport_reset_no_sock??????????????????
>
> v4.4.225: Failed to apply! Possible dependencies:
> 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?
I think we can simply remove the 't' from virtio_transport_reset_no_sock(),
the following patch should work:
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index fb2060dffb0a..17f4c93f5e75 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1104,6 +1104,14 @@ void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt)
lock_sock(sk);
+ /* Check if sk has been released before lock_sock */
+ if (sk->sk_shutdown == SHUTDOWN_MASK) {
+ (void)virtio_transport_reset_no_sock(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;
Thanks,
Stefano
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt
2020-06-05 14:29 ` Stefano Garzarella
@ 2020-06-08 3:06 ` Justin He
0 siblings, 0 replies; 7+ messages in thread
From: Justin He @ 2020-06-08 3:06 UTC (permalink / raw)
To: Stefano Garzarella, Sasha Levin; +Cc: Stefan Hajnoczi, David S. Miller, stable
> -----Original Message-----
> From: Stefano Garzarella <sgarzare@redhat.com>
> Sent: Friday, June 5, 2020 10:30 PM
> To: Sasha Levin <sashal@kernel.org>
> Cc: Justin He <Justin.He@arm.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> David S. Miller <davem@davemloft.net>; stable@vger.kernel.org
> Subject: Re: [PATCH v2] virtio_vsock: Fix race condition in
> virtio_transport_recv_pkt
>
> On Fri, Jun 05, 2020 at 02:10:49PM +0000, Sasha Levin wrote:
> > Hi
> >
> > [This is an automated email]
> >
> > This commit has been processed because it contains a -stable tag.
> > The stable tag indicates that it's relevant for the following trees: all
> >
> > The bot has tested the following trees: v5.6.15, v5.4.43, v4.19.125,
> v4.14.182, v4.9.225, v4.4.225.
> >
> > v5.6.15: Build OK!
> > v5.4.43: Build failed! Errors:
> > net/vmw_vsock/virtio_transport_common.c:1109:40:
> error: ???????????????t?????????????????? undeclared (first use in this
> function); did you mean ???????????????tm???????????????????
> > net/vmw_vsock/virtio_transport_common.c:1109:9: error: too many
> arguments to
> function ???????????????virtio_transport_reset_no_sock??????????????????
> >
> > v4.19.125: Build failed! Errors:
> > net/vmw_vsock/virtio_transport_common.c:1042:40:
> error: ???????????????t?????????????????? undeclared (first use in this
> function); did you mean ???????????????tm???????????????????
> > net/vmw_vsock/virtio_transport_common.c:1042:9: error: too many
> arguments to
> function ???????????????virtio_transport_reset_no_sock??????????????????
> >
> > v4.14.182: Build failed! Errors:
> > net/vmw_vsock/virtio_transport_common.c:1038:40:
> error: ???????????????t?????????????????? undeclared (first use in this
> function); did you mean ???????????????tm???????????????????
> > net/vmw_vsock/virtio_transport_common.c:1038:9: error: too many
> arguments to
> function ???????????????virtio_transport_reset_no_sock??????????????????
> >
> > v4.9.225: Build failed! Errors:
> > net/vmw_vsock/virtio_transport_common.c:968:40:
> error: ???????????????t?????????????????? undeclared (first use in this
> function); did you mean ???????????????tm???????????????????
> > net/vmw_vsock/virtio_transport_common.c:968:9: error: too many
> arguments to
> function ???????????????virtio_transport_reset_no_sock??????????????????
> >
> > v4.4.225: Failed to apply! Possible dependencies:
> > 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko")
> >
> >
> > NOTE: The patch will not be queued to stable trees until it is upstream.
> >
> > How should we proceed with this patch?
>
> I think we can simply remove the 't' from virtio_transport_reset_no_sock(),
> the following patch should work:
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c
> b/net/vmw_vsock/virtio_transport_common.c
> index fb2060dffb0a..17f4c93f5e75 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -1104,6 +1104,14 @@ void virtio_transport_recv_pkt(struct
> virtio_vsock_pkt *pkt)
>
> lock_sock(sk);
>
> + /* Check if sk has been released before lock_sock */
> + if (sk->sk_shutdown == SHUTDOWN_MASK) {
> + (void)virtio_transport_reset_no_sock(pkt);
> + release_sock(sk);
> + sock_put(sk);
> + goto free_pkt;
> + }
> +
Thanks Stefano,
Yes, this (t) parameter changes is introduced after 4c7246dc45e27 (v5.4-rc6-1712).
So the stable tree (version less than v5.4) should apply above patch.
Sorry for my late response, the auto mail is filtered by our outlook server ☹
--
Cheers,
Justin (Jia He)
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-08 3:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29 15:21 [PATCH v2] virtio_vsock: Fix race condition in virtio_transport_recv_pkt Jia He
2020-05-29 16:34 ` Stefano Garzarella
2020-05-30 1:14 ` Justin He
2020-05-30 1:14 ` Justin He
2020-06-05 14:10 ` Sasha Levin
2020-06-05 14:29 ` Stefano Garzarella
2020-06-08 3:06 ` Justin He
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.