vhost: fix virtqueue access lock check for handlers

Message ID 20240307103624.610080-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series vhost: fix virtqueue access lock check for handlers |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS

Commit Message

David Marchand March 7, 2024, 10:36 a.m. UTC
  As the vhost library may receive a request for an unsupported request,
it is necessary to check msg_handler before checking if locking queue
pairs is requested.

Coverity issue: 415049
Fixes: 5e8fcc60b59d ("vhost: enhance virtqueue access lock asserts")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/vhost/vhost_user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Kevin Traynor March 7, 2024, 2:20 p.m. UTC | #1
On 07/03/2024 10:36, David Marchand wrote:
> As the vhost library may receive a request for an unsupported request,
> it is necessary to check msg_handler before checking if locking queue
> pairs is requested.
> 
> Coverity issue: 415049
> Fixes: 5e8fcc60b59d ("vhost: enhance virtqueue access lock asserts")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/vhost/vhost_user.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 7fe1687f08..414192500e 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -3171,7 +3171,7 @@ vhost_user_msg_handler(int vid, int fd)
>  	 * inactive, so it is safe. Otherwise taking the access_lock
>  	 * would cause a dead lock.
>  	 */
> -	if (msg_handler->lock_all_qps) {
> +	if (msg_handler != NULL && msg_handler->lock_all_qps) {
>  		if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
>  			vhost_user_lock_all_queue_pairs(dev);
>  			unlock_required = 1;

Acked-by: Kevin Traynor <ktraynor@redhat.com>
  
Maxime Coquelin March 11, 2024, 10:42 a.m. UTC | #2
On 3/7/24 11:36, David Marchand wrote:
> As the vhost library may receive a request for an unsupported request,
> it is necessary to check msg_handler before checking if locking queue
> pairs is requested.
> 
> Coverity issue: 415049
> Fixes: 5e8fcc60b59d ("vhost: enhance virtqueue access lock asserts")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>   lib/vhost/vhost_user.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 7fe1687f08..414192500e 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -3171,7 +3171,7 @@ vhost_user_msg_handler(int vid, int fd)
>   	 * inactive, so it is safe. Otherwise taking the access_lock
>   	 * would cause a dead lock.
>   	 */
> -	if (msg_handler->lock_all_qps) {
> +	if (msg_handler != NULL && msg_handler->lock_all_qps) {
>   		if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
>   			vhost_user_lock_all_queue_pairs(dev);
>   			unlock_required = 1;

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
  
David Marchand March 13, 2024, 2:46 p.m. UTC | #3
On Thu, Mar 7, 2024 at 11:36 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> As the vhost library may receive a request for an unsupported request,
> it is necessary to check msg_handler before checking if locking queue
> pairs is requested.
>
> Coverity issue: 415049
> Fixes: 5e8fcc60b59d ("vhost: enhance virtqueue access lock asserts")
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Applied, thanks.
  

Patch

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 7fe1687f08..414192500e 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -3171,7 +3171,7 @@  vhost_user_msg_handler(int vid, int fd)
 	 * inactive, so it is safe. Otherwise taking the access_lock
 	 * would cause a dead lock.
 	 */
-	if (msg_handler->lock_all_qps) {
+	if (msg_handler != NULL && msg_handler->lock_all_qps) {
 		if (!(dev->flags & VIRTIO_DEV_VDPA_CONFIGURED)) {
 			vhost_user_lock_all_queue_pairs(dev);
 			unlock_required = 1;