linux-kernel - Re: [PATCH] VMCI: Fix NULL pointer dereference on context ptr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200318110204.GB2305113@kroah.com>
Date:   Wed, 18 Mar 2020 12:02:04 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Xiyu Yang <xiyuyang19@...an.edu.cn>
Cc:     Arnd Bergmann <arnd@...db.de>, Xin Tan <tanxin.ctf@...il.com>,
        Alexios Zavras <alexios.zavras@...el.com>,
        Vishnu DASA <vdasa@...are.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Allison Randal <allison@...utok.net>,
        Ira Weiny <ira.weiny@...el.com>,
        Mike Marshall <hubcap@...ibond.com>,
        linux-kernel@...r.kernel.org, yuanxzhang@...an.edu.cn, kjlu@....edu
Subject: Re: [PATCH] VMCI: Fix NULL pointer dereference on context ptr

On Tue, Mar 17, 2020 at 02:29:57PM +0800, Xiyu Yang wrote:
> The refcount wrapper function vmci_ctx_get() may return NULL
> context ptr. Thus, we need to add a NULL pointer check
> before its dereference.
> 
> Signed-off-by: Xiyu Yang <xiyuyang19@...an.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@...il.com>
> ---
>  drivers/misc/vmw_vmci/vmci_context.c    |  2 ++
>  drivers/misc/vmw_vmci/vmci_queue_pair.c | 17 +++++++++++------
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/misc/vmw_vmci/vmci_context.c b/drivers/misc/vmw_vmci/vmci_context.c
> index 16695366ec92..a20878fba374 100644
> --- a/drivers/misc/vmw_vmci/vmci_context.c
> +++ b/drivers/misc/vmw_vmci/vmci_context.c
> @@ -898,6 +898,8 @@ void vmci_ctx_rcv_notifications_release(u32 context_id,
>  					bool success)
>  {
>  	struct vmci_ctx *context = vmci_ctx_get(context_id);
> +	if (context == NULL)
> +		return;

Same comment as on your other patch.

And is this a v2?

>  
>  	spin_lock(&context->lock);
>  	if (!success) {
> diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> index 8531ae781195..2ecb22d08692 100644
> --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
> +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> @@ -1575,11 +1575,14 @@ static int qp_broker_attach(struct qp_broker_entry *entry,
>  		 */
>  
>  		create_context = vmci_ctx_get(entry->create_id);
> -		supports_host_qp = vmci_ctx_supports_host_qp(create_context);
> -		vmci_ctx_put(create_context);
> +		if (!create_context) {
> +			supports_host_qp =
> +				vmci_ctx_supports_host_qp(create_context);
> +			vmci_ctx_put(create_context);
>  
> -		if (!supports_host_qp)
> -			return VMCI_ERROR_INVALID_RESOURCE;
> +			if (!supports_host_qp)
> +				return VMCI_ERROR_INVALID_RESOURCE;
> +		}
>  	}
>  
>  	if ((entry->qp.flags & ~VMCI_QP_ASYMM) != (flags & ~VMCI_QP_ASYMM_PEER))
> @@ -1808,7 +1811,8 @@ static int qp_alloc_host_work(struct vmci_handle *handle,
>  		pr_devel("queue pair broker failed to alloc (result=%d)\n",
>  			 result);
>  	}
> -	vmci_ctx_put(context);
> +	if (context)
> +		vmci_ctx_put(context);

can't vmci_ctx_put() take a NULL pointer?  If not, fix this that way
please.

thanks,

greg k-h