Re: CVE-2023-52605: ACPI: extlog: fix NULL pointer dereference check

[Vegard Nossum <vegard.nossum@oracle.com>]

(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?

It might be better to just delete the NULL check altogether.

As usual, I could be wrong...


Vegard