Received-SPF: pass (google.com: domain of linux-kernel+bounces-104846-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <c9285318-affc-4d45-ba9b-40e3a86be68b@intel.com>
Date: Fri, 15 Mar 2024 20:24:11 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: CVE-2023-52605: ACPI: extlog: fix NULL pointer dereference check
To: Lee Jones <lee@kernel.org>, Prarit Bhargava <prarit@redhat.com>
CC: 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>
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>
Content-Language: en-US
From: "Wysocki, Rafael J" <rafael.j.wysocki@intel.com>
In-Reply-To: <20240314110110.GL1522089@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RUVFYnBXQWxnNSt2RTRVQjEreC9MTzJyd1F1QmJTb1JlYjFoN09ybW1wN1RW?=
 =?utf-8?B?eUxCZ3NIeWh6aC9Da3FtZnBHTzFYcVMwTUczSVYxblNJK2NaaVU3eXR3OEI0?=
 =?utf-8?B?eFV4Rmg0azRqZUU3MGFiU1pGSC9wc3hFUlAyL0tlRkJVZFVuL1hTOG1IN2h2?=
 =?utf-8?B?N3RNcW1aY1NWeG50NjlwTlZLVGg3RFlCcnhDZ09OTjBwQnFRTk02cm9POWpt?=
 =?utf-8?B?S2d4ZlFuNzFpUWhsTmxLMjlpeEtseUFSVCtGZk5hOVV1ZmFydC9IdHRudGhn?=
 =?utf-8?B?WWlpK2pLdHVDeXlFT21YQUJ2MzQ5TGJ3WUpxUmFkSEM5ZFZSL29zeEhwUnpt?=
 =?utf-8?B?RzlJa3h0Rjgvc1doekxYUGZqNkxUNFViNU1EekR1RU16OHFVSlRhWFhDWTFx?=
 =?utf-8?B?Mytwdk1vTzBxcHlybWx5NE1uVjFkcUs5bkJSNGljem5hT0w3Y0VKcGh4R0d0?=
 =?utf-8?B?RVpmM2F2Mzk3SzdkOHU3c3VFQ1BrSVRYWjQyMTI4d3BYYVlaZGRHVithYjZT?=
 =?utf-8?B?T1JHVGlaYy9YbXFQY1pWYm15V2RibGNyN3o0UmhYU3MzMnZPVmhkYk52UU5N?=
 =?utf-8?B?NExhVExDVDZUa0RpSlhOL01laEkxM1ZmaURiblhMSlhxSWhzS2JwcUNwaEtx?=
 =?utf-8?B?T2VFRFdSTmNueHhsZk15a3k2QTNmMFhMYmRHb1pZcUlDMVM2Z2M5TU5HZHlB?=
 =?utf-8?B?SjdrTVBtdHdPcXNPQnpHMmFtcWFYaVVhTC9ZY2paSmpVSWNUY3pwcUZ0WFN2?=
 =?utf-8?B?VzNKYy9rNWsyNzlSQ1dGNFRYd3k2dWREdUV6UFBuQThYbTgyTk5GU0ZYMHVh?=
 =?utf-8?B?ekVnbC81ekcvbkVZUk9SbUlXYUZVc3dwc2ROb21FdkRPMTUycitPUUE0L0cv?=
 =?utf-8?B?QkVaalllWGxiNndXYkhzSkhld2xjQWtJN2J6S2pseUhMcVIreVF3WTFOQXVq?=
 =?utf-8?B?UGR2WXZLMExBZkdYNWRaVW1pdm9Da1hjcW50WGhNb25MVVhpbHh1bDVXd1lB?=
 =?utf-8?B?K1czbUV2eDA5MlV3TUhXQzhnSEJlWjhmQWxkNkh1ekFJdnFQbElFc2QyL1cr?=
 =?utf-8?B?UUVTOVpJN2JYTU9DZU9sZlFrSitDeG1YOW8zVnBoSXRvdjNpSi94U2pnY3ZJ?=
 =?utf-8?B?ZmVNcnRCR0hVVzUyN0RYRWxUNVNyOHg4Mk9BNlZHaWVmOHEycU1nejFUaHZm?=
 =?utf-8?B?UU8vZEp0SXNZQit4YTJ1SkVzbllPN2R5U3F1NHNpTHQ3Rm1Ka0hBd2dsN2ZP?=
 =?utf-8?B?N29FZGxYNmRuQkhTQzZoVDRTeXhicHJiNlBtKy9TZUhJU01mYWNyY0Ntbkln?=
 =?utf-8?B?OWs3SStHeFlCY25tVXBkMjRNK3NHc09jK1UzSmZHWEIyc3c1TkM1RExqZlFk?=
 =?utf-8?B?M1dUZ1c4aTJJRk04SlczZlRVbzRvOHFsWG9ESlNKR0NCaVNQVHcvMzB0dzlI?=
 =?utf-8?B?aFY3ZjVubzBlWmt4d1lBZFlmTy8zd0FsSE1zWnhpZTJKcGVmYjl5a1JHcFF1?=
 =?utf-8?B?cTM1N2ZSQ0ZXYW5FY3VIQWJyV1Q2TEU3N3ZvNTJ6VkFGRklBZzFqY1VOS01p?=
 =?utf-8?B?QUNUNTBQNXRTbDBMK1hENVpmOGdUN0w5WkRSbGo0T25xeUxrZnMrVDNSUzB4?=
 =?utf-8?B?UEJYakJESkVtcWRLdU1DV3BMSEZ5ME4vZldhQXJSbjJvUktCaU14bENmRHRG?=
 =?utf-8?B?bEhkNnZuT0VRc3hSRzRETmY1N3phcFhDYnpPb3ZoaFUrRW5IcU03K3cyRjI1?=
 =?utf-8?B?SjlTUGVTcy9ZMFlFaHgvMGFqczJkdkhKUHFNbURXNG9zaWpMM3ZiTm9ZTXh6?=
 =?utf-8?B?NzU4ZXBmU1JrdkNBbmt2RXFDazFFVDk3QjFlYzd5OUgrNGloOXYxdlozUGVG?=
 =?utf-8?B?bDVtN2dsVnBZVkxhZFlZMnYwV3YydjhRbTVJRnBONXo0Q2tnU0xheFUvaC9O?=
 =?utf-8?B?VW9QTlhjQkk1L2pQUmZHOTlVbXNwcmRBbHZaaEprV1VJeUJONjRWNmtQZjQw?=
 =?utf-8?B?eHBHN1BQZXFkMHpId0JTSGxwOGd1NjcrTWZKaTBLclNHYVNZR1dRT1ArSHJx?=
 =?utf-8?B?SjNROW91U1FSM25CQ3JQMnRwVW9VZ0tvSFpYZi9ZaDIzT0Y0QmhuSm8rTWpi?=
 =?utf-8?B?U3BULzkwYjVPRDNGQ3hMSEFtVnBhQmZDOTY3cExZdHo4WmcyMkUyMWxVeWtx?=
 =?utf-8?B?bVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 636aede4-7737-482e-542e-08dc45258141
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5810.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Mar 2024 19:24:16.7463
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2LbPxiBxynzJTunUqALJoHnNo7dMn/kuK03u6f1O4+kivD8+du0JZlUvHgTXrIC0rsdZQ2/Qqul/1sX036uwrhLrliR31H/qABWP7o2DR54=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6384
X-OriginatorOrg: intel.com

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