(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
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
>
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.
--
Lee Jones [李琼斯]
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.
Thanks,
Rafael
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/[email protected]/
--
Lee Jones [李琼斯]