Received-SPF: pass (google.com: domain of linux-kernel+bounces-98812-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <7591f33e-d64f-49c5-b7c8-deda2b6f0fde@redhat.com>
Date: Mon, 11 Mar 2024 08:14:01 -0400
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: CVE-2023-52605: ACPI: extlog: fix NULL pointer dereference check
Content-Language: en-US
To: Vegard Nossum <vegard.nossum@oracle.com>, cve@kernel.org,
 linux-kernel@vger.kernel.org, linux-cve-announce@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
 "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
References: <2024030647-CVE-2023-52605-292a@gregkh>
 <e4906091-598e-4d21-9e8d-4cb088855d6f@oracle.com>
From: Prarit Bhargava <prarit@redhat.com>
In-Reply-To: <e4906091-598e-4d21-9e8d-4cb088855d6f@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

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.

P.

> 
> Vegard
>