Received-SPF: pass (google.com: domain of linux-kernel+bounces-106278-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Mon, 18 Mar 2024 15:13:23 +0000
From: Lee Jones <lee@kernel.org>
To: "Wysocki, Rafael J" <rafael.j.wysocki@intel.com>
Cc: Prarit Bhargava <prarit@redhat.com>,
	Vegard Nossum <vegard.nossum@oracle.com>, cve@kernel.org,
	linux-kernel@vger.kernel.org, linux-cve-announce@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: CVE-2023-52605: ACPI: extlog: fix NULL pointer dereference check
Message-ID: <20240318151323.GS1522089@google.com>
References: <2024030647-CVE-2023-52605-292a@gregkh>
 <e4906091-598e-4d21-9e8d-4cb088855d6f@oracle.com>
 <7591f33e-d64f-49c5-b7c8-deda2b6f0fde@redhat.com>
 <20240314110110.GL1522089@google.com>
 <c9285318-affc-4d45-ba9b-40e3a86be68b@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <c9285318-affc-4d45-ba9b-40e3a86be68b@intel.com>

On Fri, 15 Mar 2024, Wysocki, Rafael J wrote:

> On 3/14/2024 12:01 PM, Lee Jones wrote:
> > On Mon, 11 Mar 2024, Prarit Bhargava wrote:
> > 
> > > On 3/10/24 04:10, Vegard Nossum wrote:
> > > > (Added author/maintainer to Cc)
> > > > 
> > > > On 06/03/2024 07:46, Greg Kroah-Hartman wrote:
> > > > > Description
> > > > > ===========
> > > > > 
> > > > > In the Linux kernel, the following vulnerability has been resolved:
> > > > > 
> > > > > ACPI: extlog: fix NULL pointer dereference check
> > > > > 
> > > > > The gcc plugin -fanalyzer [1] tries to detect various
> > > > > patterns of incorrect behaviour.  The tool reports:
> > > > > 
> > > > > drivers/acpi/acpi_extlog.c: In function ‘extlog_exit’:
> > > > > drivers/acpi/acpi_extlog.c:307:12: warning: check of
> > > > > ‘extlog_l1_addr’ for NULL after already dereferencing it
> > > > > [-Wanalyzer-deref-before-check]
> > > > >       |
> > > > >       |  306 |         ((struct extlog_l1_head
> > > > > *)extlog_l1_addr)->flags &= ~FLAG_OS_OPTIN;
> > > > >       |      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
> > > > >       |      |                                                  |
> > > > >       |      |                                                  (1)
> > > > > pointer ‘extlog_l1_addr’ is dereferenced here
> > > > >       |  307 |         if (extlog_l1_addr)
> > > > >       |      |            ~
> > > > >       |      |            |
> > > > >       |      |            (2) pointer ‘extlog_l1_addr’ is checked for
> > > > > NULL here but it was already dereferenced at (1)
> > > > >       |
> > > > > 
> > > > > Fix the NULL pointer dereference check in extlog_exit().
> > > > > 
> > > > > The Linux kernel CVE team has assigned CVE-2023-52605 to this issue.
> > > > This code is in an __exit function:
> > > > 
> > > > diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> > > > index e120a96e1eaee..193147769146e 100644
> > > > --- a/drivers/acpi/acpi_extlog.c
> > > > +++ b/drivers/acpi/acpi_extlog.c
> > > > @@ -303,9 +303,10 @@ err:
> > > >    static void __exit extlog_exit(void)
> > > >    {
> > > >        mce_unregister_decode_chain(&extlog_mce_dec);
> > > > -    ((struct extlog_l1_head *)extlog_l1_addr)->flags &= ~FLAG_OS_OPTIN;
> > > > -    if (extlog_l1_addr)
> > > > +    if (extlog_l1_addr) {
> > > > +        ((struct extlog_l1_head *)extlog_l1_addr)->flags &=
> > > > ~FLAG_OS_OPTIN;
> > > >            acpi_os_unmap_iomem(extlog_l1_addr, l1_size);
> > > > +    }
> > > >        if (elog_addr)
> > > >            acpi_os_unmap_iomem(elog_addr, elog_size);
> > > >        release_mem_region(elog_base, elog_size);
> > > > 
> > > > This can only run when you unload a module, which is a privileged
> > > > operation (restricted to CAP_SYS_MODULE).
> > > > 
> > > > Moreover, extlog_l1_addr is only ever assigned in the corresponding
> > > > module init function, and it looks like it will never be NULL if the
> > > > module was loaded successfully, at least on a recent mainline kernel.
> > > > 
> > > > Since the module exit won't be called unless module init succeeded, I
> > > > don't see a way to trigger this bug. Is this a vulnerability?
> > > > 
> > > This is certainly not a CVE.
> > > 
> > > > It might be better to just delete the NULL check altogether.
> > > > 
> > > > As usual, I could be wrong...
> > > > 
> > > When I made this code change I thought the same thing: Perhaps it's better
> > > to remove the NULL check given the status of the code.  I assumed that the
> > > check was there as a failsafe on unload.
> > If Rafael agrees with you both, I'd be happy to revoke its CVE status.
> 
> I do agree with the analysis above, sorry for the delay.

No problem Rafael.

Okay, CVE-2023-52605 has now been rejected.

https://lore.kernel.org/all/20240318151025.2952751-2-lee@kernel.org/

-- 
Lee Jones [李琼斯]