[PATCH] acpi: acpica: fix acpi operand cache leak

[Seunghun Han]

I'm Seunghun Han, and I work for National Security Research Institute of
South Korea.

I have been doing a research on ACPI and making a handcrafted ACPI table
for my research.
Errors of handcrafted ACPI tables are handled well in Linux kernel while boot
process, and Linux kernel goes well without critical problems.
But I found some ACPI operand cache leaks in ACPI early abort cases.

Boot log of ACPI operand cache leak is as follows:
>[ 0.174332] ACPI: Added _OSI(Module Device)
>[ 0.175504] ACPI: Added _OSI(Processor Device)
>[ 0.176010] ACPI: Added _OSI(3.0 _SCP Extensions)
>[ 0.177032] ACPI: Added _OSI(Processor Aggregator Device)
>[ 0.178284] ACPI: SCI (IRQ16705) allocation failed
>[ 0.179352] ACPI Exception: AE_NOT_ACQUIRED, Unable to install System Control Interrupt handler (20160930/evevent-131)
>[ 0.180008] ACPI: Unable to start the ACPI Interpreter
>[ 0.181125] ACPI Error: Could not remove SCI handler (20160930/evmisc-281)
>[ 0.184068] kmem_cache_destroy Acpi-Operand: Slab cache still has objects
>[ 0.185358] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.10.0-rc3 #2
>[ 0.186820] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>[ 0.188000] Call Trace:
>[ 0.188000] ? dump_stack+0x5c/0x7d
>[ 0.188000] ? kmem_cache_destroy+0x224/0x230
>[ 0.188000] ? acpi_sleep_proc_init+0x22/0x22
>[ 0.188000] ? acpi_os_delete_cache+0xa/0xd
>[ 0.188000] ? acpi_ut_delete_caches+0x3f/0x7b
>[ 0.188000] ? acpi_terminate+0x5/0xf
>[ 0.188000] ? acpi_init+0x288/0x32e
>[ 0.188000] ? __class_create+0x4c/0x80
>[ 0.188000] ? video_setup+0x7a/0x7a
>[ 0.188000] ? do_one_initcall+0x4e/0x1b0
>[ 0.188000] ? kernel_init_freeable+0x194/0x21a
>[ 0.188000] ? rest_init+0x80/0x80
>[ 0.188000] ? kernel_init+0xa/0x100
>[ 0.188000] ? ret_from_fork+0x25/0x30

When early abort is occurred due to invalid ACPI information, Linux kernel
terminates ACPI by calling acpi_terminate() function.
The function calls acpi_ns_terminate() function to delete namespace data
and ACPI operand cache (acpi_gbl_module_code_list).

But the deletion code in acpi_ns_terminate() function is wrapped in
ACPI_EXEC_APP definition, therefore the code is only executed when the
definition exists.
If the define doesn't exist, ACPI operand cache (acpi_gbl_module_code_list) is
leaked, and stack dump is shown in kernel log.

This causes a security threat because the old kernel (<= 4.9) shows memory
locations of kernel functions in stack dump, therefore kernel ASLR can be
neutralized.

To fix ACPI operand leak for enhancing security, I made a patch which removes
the ACPI_EXEC_APP define in acpi_ns_terminate() function for executing the
deletion code unconditionally.

I hope that this patch improves the security of Linux kernel.

Thank you.

Signed-off-by: Seunghun Han <kkam...@gmail.com>
---
drivers/acpi/acpica/nsutils.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/acpica/nsutils.c b/drivers/acpi/acpica/nsutils.c
index 691814d..acb6099 100644
--- a/drivers/acpi/acpica/nsutils.c
+++ b/drivers/acpi/acpica/nsutils.c
@@ -597,22 +597,18 @@ void acpi_ns_terminate(void)

ACPI_FUNCTION_TRACE(ns_terminate);

-#ifdef ACPI_EXEC_APP
- {
- union acpi_operand_object *prev;
- union acpi_operand_object *next;
-
- /* Delete any module-level code blocks */
-
- next = acpi_gbl_module_code_list;
- while (next) {
- prev = next;
- next = next->method.mutex;
- prev->method.mutex = NULL; /* Clear the Mutex (cheated) field */
- acpi_ut_remove_reference(prev);
- }
+ union acpi_operand_object *prev;
+ union acpi_operand_object *next;
+
+ /* Delete any module-level code blocks */
+
+ next = acpi_gbl_module_code_list;
+ while (next) {
+ prev = next;
+ next = next->method.mutex;
+ prev->method.mutex = NULL; /* Clear the Mutex (cheated) field */
+ acpi_ut_remove_reference(prev);
}
-#endif

/*
* Free the entire namespace -- all nodes and all objects
--
2.1.4

[Seunghun Han]

Changes since v1: move position of variables to remove compile warning.

drivers/acpi/acpica/nsutils.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/acpica/nsutils.c b/drivers/acpi/acpica/nsutils.c
index 691814d..943702d 100644
--- a/drivers/acpi/acpica/nsutils.c
+++ b/drivers/acpi/acpica/nsutils.c
@@ -594,25 +594,20 @@ struct acpi_namespace_node *acpi_ns_validate_handle(acpi_handle handle)
void acpi_ns_terminate(void)
{
acpi_status status;

+ union acpi_operand_object *prev;
+ union acpi_operand_object *next;

ACPI_FUNCTION_TRACE(ns_terminate);

-#ifdef ACPI_EXEC_APP
- {
- union acpi_operand_object *prev;
- union acpi_operand_object *next;

+ /* Delete any module-level code blocks */

- /* Delete any module-level code blocks */
-
- next = acpi_gbl_module_code_list;
- while (next) {
- prev = next;
- next = next->method.mutex;
- prev->method.mutex = NULL; /* Clear the Mutex (cheated) field */
- acpi_ut_remove_reference(prev);
- }

[Moore, Robert]

I'm sure we would like to backport the memory leak into ACPICA code.

Not sure about the warnings.

> -----Original Message-----
> From: lkp
> Sent: Saturday, February 11, 2017 7:56 PM
> To: Seunghun Han <kkam...@gmail.com>
> Cc: kbuil...@01.org; Zheng, Lv <lv.z...@intel.com>; Moore, Robert
> <robert...@intel.com>; Wysocki, Rafael J
> <rafael.j...@intel.com>; linux-...@vger.kernel.org; Seunghun Han
> <kkam...@gmail.com>
> Subject: Re: [PATCH] acpi: acpica: fix acpi operand cache leak
>
> Hi Seunghun,
>
> [auto build test WARNING on pm/linux-next] [also build test WARNING on
> v4.10-rc7 next-20170210] [if your patch is applied to the wrong git
> tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Seunghun-Han/acpi-
> acpica-fix-acpi-operand-cache-leak/20170212-105735
> base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-
> pm.git linux-next
> config: i386-randconfig-x003-201707 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All warnings (new ones prefixed by >>):
>
> drivers/acpi/acpica/nsutils.c: In function 'acpi_ns_terminate':
> >> drivers/acpi/acpica/nsutils.c:600:2: warning: ISO C90 forbids mixed
> >> declarations and code [-Wdeclaration-after-statement]
> union acpi_operand_object *prev;
> ^~~~~
>
> vim +600 drivers/acpi/acpica/nsutils.c
>
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 584
> * FUNCTION: acpi_ns_terminate
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 585
> *
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 586
> * PARAMETERS: none
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 587
> *
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 588
> * RETURN: none
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 589
> *
> 44f6c012 drivers/acpi/namespace/nsutils.c Robert Moore 2005-04-18 590
> * DESCRIPTION: free memory allocated for namespace and ACPI table
> storage.
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 591
> *
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 592
> ************************************************************************
> ******/
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 593
> 4be44fcd drivers/acpi/namespace/nsutils.c Len Brown 2005-08-05 594
> void acpi_ns_terminate(void)
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 595
> {
> 3f69fe15 drivers/acpi/acpica/nsutils.c Bob Moore 2013-11-21 596
> acpi_status status;
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 597
> b229cf92 drivers/acpi/namespace/nsutils.c Bob Moore 2006-04-21 598
> ACPI_FUNCTION_TRACE(ns_terminate);
> ^1da177e drivers/acpi/namespace/nsutils.c Linus Torvalds 2005-04-16 599
> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 @600
> union acpi_operand_object *prev;
> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 601
> union acpi_operand_object *next;
> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 602
> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 603

> /* Delete any module-level code blocks */

> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 604
> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 605
> next = acpi_gbl_module_code_list;
> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 606
> while (next) {
> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 607
> prev = next;
> 25823e78 drivers/acpi/acpica/nsutils.c Bob Moore 2015-08-25 608
> next = next->method.mutex;
>
> :::::: The code at line 600 was first introduced by commit
> :::::: 25823e784aac78964ada0e49efe2766d2aeb9fa4 ACPICA: Add additional
> debug info/statements
>
> :::::: TO: Bob Moore <robert...@intel.com>
> :::::: CC: Rafael J. Wysocki <rafael.j...@intel.com>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all Intel
> Corporation

[Seung Hun Han]

Thank you for your reply.

According to your opinion, I made and sent a patch v2 email to you.
The patch v2 removed all warnings of kbuild by moving the position of
variables.

I extracted the patch v2 from the email.
So would you check the patch v2 under this email or the patch v2 email
in your email list?

The patch v2 is as follows.

Signed-off-by: Seunghun Han <kkam...@gmail.com>
---

Changes since v1: move position of variables to remove compile warning.

drivers/acpi/acpica/nsutils.c | 23 +++++++++--------------

1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/acpica/nsutils.c b/drivers/acpi/acpica/nsutils.c
index 691814d..943702d 100644
--- a/drivers/acpi/acpica/nsutils.c
+++ b/drivers/acpi/acpica/nsutils.c

@@ -594,25 +594,20 @@ struct acpi_namespace_node
*acpi_ns_validate_handle(acpi_handle handle)
void acpi_ns_terminate(void)
{
acpi_status status;

+ union acpi_operand_object *prev;
+ union acpi_operand_object *next;

ACPI_FUNCTION_TRACE(ns_terminate);

-#ifdef ACPI_EXEC_APP
- {
- union acpi_operand_object *prev;
- union acpi_operand_object *next;

+ /* Delete any module-level code blocks */

- /* Delete any module-level code blocks */
-
- next = acpi_gbl_module_code_list;
- while (next) {
- prev = next;
- next = next->method.mutex;
- prev->method.mutex = NULL; /* Clear the Mutex (cheated) field */
- acpi_ut_remove_reference(prev);
- }

+ next = acpi_gbl_module_code_list;
+ while (next) {
+ prev = next;
+ next = next->method.mutex;
+ prev->method.mutex = NULL; /* Clear the Mutex (cheated) field */
+ acpi_ut_remove_reference(prev);
}
-#endif

/*
* Free the entire namespace -- all nodes and all objects
--
2.1.4

[Seung Hun Han]

Hi, Robert.

I'm so sorry for bothering you. My email client ignored an indentation of the
patch file.
Therefore, please check my patch v2 in your email list, "[PATCH v2] acpi:
acpica: fix acpi operand cache leak" (https://lkml.org/lkml/2017/2/12/224).

Thank you.

[Rafael J. Wysocki]

On Friday, February 24, 2017 08:52:42 PM Seunghun Han wrote:
> Hi, Lv Zheng.
>
> I added my handcrafted ACPI table under your request, because
> "acpidump -c on" and "acpidump -c off" doesn't work.
>
> 2017-02-21 19:36 GMT+09:00 Seunghun Han <kkam...@gmail.com>:
> > Hello,
> >
> > I attached the test results below,
> >
> > 2017-02-21 9:53 GMT+09:00 Rowafael J. Wysocki <r...@rjwysocki.net>:
> >> On Tuesday, February 21, 2017 12:33:08 AM Zheng, Lv wrote:
> >>> Hi,

> >>> I'm more interested in the way of triggering AE_NOT_ACQUIRED error.
> >>> So could you send us the handcrafted ACPI table or both the "acpidump -c on" and "acpidump -c off" output?
>
> I modified FACP, FACS, APIC table in VirtualBox for Linux.
> Here are raw dumps of table.

So, excuse me, but what's the security issue here?

You hacked your ACPI tables into pieces which requires root privileges anyway.

Thanks,
Rafael

[Seunghun Han]

[ 0.000000] ACPI: FACP 0x00000000DFFF00F0 0000F4 (v04 VBOX
VBOXFACP 00000001 ASL 00000061)
[ 0.000000] FACP DUMP
[ 0.000000] 0x0000: 46 41 43 50 F4 00 00 00 04 60 56 42 4F 58 20 20
[ 0.000000] 0x0010: 56 42 4F 58 46 41 43 50 01 00 00 00 41 53 4C 20
[ 0.000000] 0x0020: 61 00 00 00 00 02 FF DF 80 04 FF DF 41 41 41 41
[ 0.000000] 0x0030: 2E 44 00 00 A1 A0 00 00 00 40 00 00 00 00 00 00
[ 0.000000] 0x0040: 04 40 00 00 00 00 00 00 00 00 00 00 08 40 00 00
[ 0.000000] 0x0050: 20 40 00 00 00 00 00 00 04 02 00 04 02 00 00 00
[ 0.000000] 0x0060: 65 00 E9 03 00 00 00 00 00 00 00 00 00 03 00 00
[ 0.000000] 0x0070: 41 05 00 00 01 08 00 01 50 40 00 00 00 00 00 00
[ 0.000000] 0x0080: 10 00 00 00 00 02 FF DF 00 00 00 00 80 04 FF DF
[ 0.000000] 0x0090: 00 00 00 00 01 20 00 02 00 40 00 00 00 00 00 00
[ 0.000000] 0x00A0: 00 00 00 00 00 00 00 00 00 00 00 00 01 10 00 02
[ 0.000000] 0x00B0: 04 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 0.000000] 0x00C0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 0.000000] 0x00D0: 01 20 00 03 08 40 00 00 00 00 00 00 01 10 00 01
[ 0.000000] 0x00E0: 20 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 0.000000] 0x00F0: 00 00 00 00

[ 0.000000] ACPI: FACS 0x00000000DFFF0200 000040
[ 0.000000] FACS DUMP
[ 0.000000] 0x0000: 46 41 43 53 40 00 00 00 00 00 00 00 00 00 00 00
[ 0.000000] 0x0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 0.000000] 0x0020: 01 00 00 00 00 00 00 00 00 41 00 00 00 00 00 00
[ 0.000000] 0x0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

[ 0.000000] ACPI: FACS 0x00000000DFFF0200 000040
[ 0.000000] FACS DUMP
[ 0.000000] 0x0000: 46 41 43 53 40 00 00 00 00 00 00 00 00 00 00 00
[ 0.000000] 0x0010: 00 00 00 00 00 00 00 00 00 41 41 41 41 41 41 41
[ 0.000000] 0x0020: 01 00 00 00 00 00 00 00 00 41 00 00 00 00 00 00
[ 0.000000] 0x0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

[ 0.000000] ACPI: APIC 0x00000000DFFF0240 00006C (v02 VBOX
VBOXAPIC 00000001 ASL 00000061)
[ 0.000000] APIC DUMP
[ 0.000000] 0x0000: 41 50 49 43 6C 00 00 00 02 21 56 42 4F 58 20 20
[ 0.000000] 0x0010: 56 42 4F 58 41 50 49 43 01 00 00 00 41 53 4C 20
[ 0.000000] 0x0020: 61 00 00 00 00 00 E0 FE 01 00 00 00 02 0A 00 00
[ 0.000000] 0x0030: 02 00 00 00 00 00 02 0A 00 09 09 00 00 00 0D 00
[ 0.000000] 0x0040: 00 08 00 00 01 00 41 41 41 41 41 41 41 41 41 00
[ 0.000000] 0x0050: 00 08 02 02 01 00 00 00 00 08 03 03 01 00 00 00
[ 0.000000] 0x0060: 01 0C 04 00 00 00 C0 FE 00 00 00 00

If you need additional data, please let me know.
Thank you.

Best regards.

>
> Because of the ACPI interpreter error, ACPI function were terminated,
> so there is no directory "/proc/acpi".
> And when I typed the acpidump command, errors were shown.
>
> The error are as follows.
> root@debian:/proc# acpidump -c on
> Cannot open directory - /sys/firmware/acpi/tables
> Could not get ACPI tables, AE_NOT_FOUND
>
> root@debian:/proc# acpidump -c off
> Cannot open directory - /sys/firmware/acpi/tables
> Could not get ACPI tables, AE_NOT_FOUND
>
> Could you tell me another way to get information for you?
> Thank you.
>
> Best regards.

>
>>> >
>>> > When early abort is occurred due to invalid ACPI information, Linux kernel
>>> > terminates ACPI by calling acpi_terminate() function.
>>> > The function calls acpi_ns_terminate() function to delete namespace data
>>> > and ACPI operand cache (acpi_gbl_module_code_list).
>>> >
>>> > But the deletion code in acpi_ns_terminate() function is wrapped in
>>> > ACPI_EXEC_APP definition, therefore the code is only executed when the
>>> > definition exists.
>>> > If the define doesn't exist, ACPI operand cache (acpi_gbl_module_code_list) is
>>> > leaked, and stack dump is shown in kernel log.
>>> >
>>>

>>> acpi_ns_terminate() actually shouldn't be invoked by Linux.
>>> It's not fully functioning in Linux kernel environment.

>>>
>>> > This causes a security threat because the old kernel (<= 4.9) shows memory
>>> > locations of kernel functions in stack dump, therefore kernel ASLR can be
>>> > neutralized.
>>> >
>>> > To fix ACPI operand leak for enhancing security, I made a patch which removes
>>> > the ACPI_EXEC_APP define in acpi_ns_terminate() function for executing the
>>> > deletion code unconditionally.
>>>

>>> However acpi_gbl_module_code_list deletion shouldn't be dependent on ACPI_EXEC_APP.
>>> So your change is acceptable.

>>>
>>> >
>>> > I hope that this patch improves the security of Linux kernel.
>>> >
>>> > Thank you.
>>> >

>>> Thus this looks OK to me.

>>>
>>> >
>>> > /*
>>> > * Free the entire namespace -- all nodes and all objects
>>> > --
>>> > 2.1.4
>>

>> I still would prefer it to go in via the upstream.
>>
>> Thanks,
>> Rafael
>>

[Rafael J. Wysocki]

On Friday, February 24, 2017 09:15:52 PM Seunghun Han wrote:
> Hi, Rafael.
>
> I added my opinion below.

> As you mentioned earlier, I hacked my ACPI table for research, so it seems that
> it is not a security issue.
>
> But, if new mainboard are released and they have a vendor-specific ACPI table
> which has invalid data, the old version of kernel (<=4.9) will possibly expose
> kernel address and KASLR will be neutralized unintentionally.

But that would mean a basically non-functional system, so I'm not sure how
anyone can actually take advantage of the "KASLR neutralization".

> I know the vendors collaborate with Linux kernel developers, but the problem
> can still occur.
>
> Hardware vendors release so many kinds of mainboard in a year, and the major
> Linux distros (Ubuntu, Fedora, etc.) will have 4.8 kernel for a long time.
>
> For this reason, I think this issue has a security aspect.

Well, not quite IMO.

If the system needs ACPI tables and the kernel cannot use them, it just won't
work no matter what.

Thanks,
Rafael

[Seunghun Han]

Hi, Rafael.

I added my opinion below.

2017-02-24 20:50 GMT+09:00 Rafael J. Wysocki <r...@rjwysocki.net>:

As you mentioned earlier, I hacked my ACPI table for research, so it seems that
it is not a security issue.

But, if new mainboard are released and they have a vendor-specific ACPI table
which has invalid data, the old version of kernel (<=4.9) will possibly expose
kernel address and KASLR will be neutralized unintentionally.

I know the vendors collaborate with Linux kernel developers, but the problem
can still occur.

Hardware vendors release so many kinds of mainboard in a year, and the major
Linux distros (Ubuntu, Fedora, etc.) will have 4.8 kernel for a long time.

For this reason, I think this issue has a security aspect.

Thank you.

Best regards.

[Seunghun Han]

Hi, Rafeal.

I added my opinion below.

I think an attacker can take advantage of the "KASLR neutralization". As you
know, KASLR is good technology to protect kernel from kernel exploits.

If the kernel has vulnerabilities, the attacker can make exploit using them.
But, the exploit usually needs gadgets (small code), therefore the attacker
should know where the gadgets are in kernel. If the KASLR is working in kernel,
the attacker should find the actual kernel address, and he can get kernel
address information from kernel warning.

>
>> I know the vendors collaborate with Linux kernel developers, but the problem
>> can still occur.
>>
>> Hardware vendors release so many kinds of mainboard in a year, and the major
>> Linux distros (Ubuntu, Fedora, etc.) will have 4.8 kernel for a long time.
>>
>> For this reason, I think this issue has a security aspect.
>
> Well, not quite IMO.
>
> If the system needs ACPI tables and the kernel cannot use them, it just won't
> work no matter what.
>
> Thanks,
> Rafael
>

Yes, you are right. But, Linux kernel has well-defined exception handlers, so
some systems may work fine like my test machine. Moreover the users may not
recognize what the problem is, and I think that they will use the system in
insecure status for a long time.

Thank you.
Best regards.

[Rafael J. Wysocki]

If the system basically doesn't work, that information isn't particularly useful.

> >> I know the vendors collaborate with Linux kernel developers, but the problem
> >> can still occur.
> >>
> >> Hardware vendors release so many kinds of mainboard in a year, and the major
> >> Linux distros (Ubuntu, Fedora, etc.) will have 4.8 kernel for a long time.
> >>
> >> For this reason, I think this issue has a security aspect.
> >
> > Well, not quite IMO.
> >
> > If the system needs ACPI tables and the kernel cannot use them, it just won't
> > work no matter what.
> >
> > Thanks,
> > Rafael
> >
> Yes, you are right. But, Linux kernel has well-defined exception handlers, so
> some systems may work fine like my test machine. Moreover the users may not
> recognize what the problem is, and I think that they will use the system in
> insecure status for a long time.

A virtual box or a guest can run without ACPI tables. A bare metal system
where ACPI tables are necessary will be more-or-less unusable if the kernel
cannot use them (it won't be able to detect interrupt controllers and the PCI
host bridge just for starters).

Running a guest with totally broken ACPI tables requires root privileges on the
host. Running a bare metal system with totally broken ACPI tables (which seems
to be your basic concern) may be a good research project, but nobody will do
that in practice. And everybody who tries that will notice what's going on.

Yes, you found a bug, but I still am not convinced about how this is security-related.

Thanks,
Rafael

[Seunghun Han]

Hi, Rafael.

I agree with you and I added my opinion below.

I totally agree with you that this case is not in practice now.
I just started researching on ACPI, and I don't have enough ideas to occur
a security problem using a bug. I just think that it has a little possibility
which is security-related.

Thank you so much for your guides.
It helps me a lot to change my research direction.

So, could my patch be merged in next kernel (4.11 rc-1)? or do I need to do
something for it?
Please let me know.

Best regards.

>
> Thanks,
> Rafael
>

[Rafael J. Wysocki]

Generally, ACPICA patches (and this is one of them) have to go in via
the upstream ACPICA project maintained by Bob Moore and Lv. Please
see MAINTAINERS for pointers to the mailing list etc.

Lv, can you please advise on the next steps?

Thanks,
Rafael

[Zheng, Lv]

Hi, Rafael

I already gave my advices.
The fix was OK to me and I back ported it to ACPICA:
https://github.com/acpica/acpica/pull/206
However it fixes a code path that in theory shouldn't be invoked in Linux kernel.
But anyway it was merged and you will see it in the next ACPICA release.

I asked Han for the handcrafted ACPI table.
And obtained that:

ACPI: FACP 0x00000000DFFF00F0 0000F4 (v04 VBOX VBOXFACP 00000001 ASL 00000061)

0x0000: 46 41 43 50 F4 00 00 00 04 60 56 42 4F 58 20 20

0x0010: 56 42 4F 58 46 41 43 50 01 00 00 00 41 53 4C 20

0x0020: 61 00 00 00 00 02 FF DF 80 04 FF DF 41 41 41 41

0x0030: 2E 44 00 00 A1 A0 00 00 00 40 00 00 00 00 00 00

0x0040: 04 40 00 00 00 00 00 00 00 00 00 00 08 40 00 00

0x0050: 20 40 00 00 00 00 00 00 04 02 00 04 02 00 00 00

0x0060: 65 00 E9 03 00 00 00 00 00 00 00 00 00 03 00 00

0x0070: 41 05 00 00 01 08 00 01 50 40 00 00 00 00 00 00

0x0080: 10 00 00 00 00 02 FF DF 00 00 00 00 80 04 FF DF

0x0090: 00 00 00 00 01 20 00 02 00 40 00 00 00 00 00 00

0x00A0: 00 00 00 00 00 00 00 00 00 00 00 00 01 10 00 02

0x00B0: 04 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00

0x00C0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

0x00D0: 01 20 00 03 08 40 00 00 00 00 00 00 01 10 00 01

0x00E0: 20 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00

0x00F0: 00 00 00 00

ACPI: FACS 0x00000000DFFF0200 000040

0x0000: 46 41 43 53 40 00 00 00 00 00 00 00 00 00 00 00

0x0010: 00 00 00 00 00 00 00 00 00 41 41 41 41 41 41 41

0x0020: 01 00 00 00 00 00 00 00 00 41 00 00 00 00 00 00

0x0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

ACPI: APIC 0x00000000DFFF0240 00006C (v02 VBOX VBOXAPIC 00000001 ASL 00000061)

0x0000: 41 50 49 43 6C 00 00 00 02 21 56 42 4F 58 20 20

0x0010: 56 42 4F 58 41 50 49 43 01 00 00 00 41 53 4C 20

0x0020: 61 00 00 00 00 00 E0 FE 01 00 00 00 02 0A 00 00

0x0030: 02 00 00 00 00 00 02 0A 00 09 09 00 00 00 0D 00

0x0040: 00 08 00 00 01 00 41 41 41 41 41 41 41 41 41 00

0x0050: 00 08 02 02 01 00 00 00 00 08 03 03 01 00 00 00

0x0060: 01 0C 04 00 00 00 C0 FE 00 00 00 00

Where there is still no AML tables and the failure in the patch description seems to be related to the AML tables.
So I'm still not aware of what the "handcrafted tables" meant to us and what the problem was.

Thanks and best regards
Lv

>
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majo...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

[Rafael J. Wysocki]

Thank you!

Best,
Rafael

[Seunghun Han]

Hello, Lv and Rafael.

I checked that my patch was merged to ACPICA project.
Thank you for your notice.

I added an analysis report which has the root cause of this problem below.

Actually, I did not change DSDT and SSDT. I changed only FACP, FACS and APIC for
my handcrafted ACPI table.

I have analyzed the root cause of the problem, and I have found that my
handcrafted ACPI table has too big SCI IRQ (like 16705).
So, acpi_os_install_interrupt_handler() which is called by
acpi_enable_subsystem()
was failed and returned AE_NOT_ACQUIRED (0x14) with "ACPI: SCI (IRQ16705)
allocation failed" log.

Because of error code, acpi_bus_init(), the caller of acpi_enable_subsystem(),
showed "Unable to start the ACPI Interpreter" and called acpi_terminate() for
exception handling. After calling acpi_terminate(), as you know, cache leak
occurred.
This means that error of acpi_load_tables(), acpi_initialize_objects(),
acpi_bus_init_irq() and acpi_install_notify_handler() which are called by
acpi_bus_init() can cause cache leak.

If you want to see this error handling sequence, I suggest you that you change
the code of acpi_os_install_interrupt_handler() to return AE_NOT_ACQUIRED
immediately. Then, you can see the error handling sequence without my
handcrafted
ACPI table.

I you want additional information about this, please let me know.

Best regards.

>

[Zheng, Lv]

Hi,

Thanks for the explanation.
There doesn't seem to be any required additional fix for such a wrong sci irq #.
Which can release me from the triggered error now. :)

>
> Because of error code, acpi_bus_init(), the caller of acpi_enable_subsystem(),
> showed "Unable to start the ACPI Interpreter" and called acpi_terminate() for
> exception handling. After calling acpi_terminate(), as you know, cache leak
> occurred.
> This means that error of acpi_load_tables(), acpi_initialize_objects(),
> acpi_bus_init_irq() and acpi_install_notify_handler() which are called by
> acpi_bus_init() can cause cache leak.
>
> If you want to see this error handling sequence, I suggest you that you change
> the code of acpi_os_install_interrupt_handler() to return AE_NOT_ACQUIRED
> immediately. Then, you can see the error handling sequence without my
> handcrafted
> ACPI table.

You can always harden the code with acceptable improvements.
Feel free to continue your contribution.

>
> I you want additional information about this, please let me know.

Sure.

Thanks
Lv