linux-kernel - Re: 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: <20200323065506.GA131098@kroah.com>
Date:   Mon, 23 Mar 2020 07:55:06 +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: Re: [PATCH] VMCI: Fix NULL pointer dereference on context ptr

On Mon, Mar 23, 2020 at 12:53:02PM +0800, Xiyu Yang wrote:
> Hi Greg,
> 
> On Wed, Mar 18, 2020 at 12:02:04PM +0100, Greg Kroah-Hartman wrote:
> > 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?
> 
> Thanks! Yes, this is a v2. 
> 
> According  to our observation, vmci_ctx_rcv_notifications_release() 
> currently is only called by vmci_host_do_recv_notifications() which 
> guarantees a valid context object can be acquired with this context_id. 
> 
> However, we argue that a NULL-check here is still necessary because 
> this function may be called by other functions in the future who may 
> fail/forget to provide such guarantee. 

No, that's not how we write code in the kernel, if it does not need to
be checked for because this can not happen, then do not check for it.

Don't try to plan for random users of your code in the future when you
control those users directly :)

thanks,

greg k-h