acpi: acpica: fix acpi parse and parseext cache leaks

kkamagui · kkamagui · commit 8829e70e1360 · 2018-04-04T02:59:28.000+09:00
I'm Seunghun Han, and I work for National Security Research Institute of South Korea. I have been doing a research on ACPI and found an ACPI cache leak in ACPI early abort cases. Boot log of ACPI cache leak is as follows: [ 0.352414] ACPI: Added _OSI(Module Device) [ 0.353182] ACPI: Added _OSI(Processor Device) [ 0.353182] ACPI: Added _OSI(3.0 _SCP Extensions) [ 0.353182] ACPI: Added _OSI(Processor Aggregator Device) [ 0.356028] ACPI: Unable to start the ACPI Interpreter [ 0.356799] ACPI Error: Could not remove SCI handler (20170303/evmisc-281) [ 0.360215] kmem_cache_destroy Acpi-State: Slab cache still has objects [ 0.360648] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 4.12.0-rc4-next-20170608+ #10 [ 0.361273] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 0.361873] Call Trace: [ 0.362243] ? dump_stack+0x5c/0x81 [ 0.362591] ? kmem_cache_destroy+0x1aa/0x1c0 [ 0.362944] ? acpi_sleep_proc_init+0x27/0x27 [ 0.363296] ? acpi_os_delete_cache+0xa/0x10 [ 0.363646] ? acpi_ut_delete_caches+0x6d/0x7b [ 0.364000] ? acpi_terminate+0xa/0x14 [ 0.364000] ? acpi_init+0x2af/0x34f [ 0.364000] ? __class_create+0x4c/0x80 [ 0.364000] ? video_setup+0x7f/0x7f [ 0.364000] ? acpi_sleep_proc_init+0x27/0x27 [ 0.364000] ? do_one_initcall+0x4e/0x1a0 [ 0.364000] ? kernel_init_freeable+0x189/0x20a [ 0.364000] ? rest_init+0xc0/0xc0 [ 0.364000] ? kernel_init+0xa/0x100 [ 0.364000] ? ret_from_fork+0x25/0x30 I analyzed this memory leak in detail. I found that “Acpi-State” cache and “Acpi-Parse” cache were merged because the size of cache objects was same slab cache size. I finally found “Acpi-Parse” cache and “Acpi-ParseExt” cache were leaked using SLAB_NEVER_MERGE flag in kmem_cache_create() function. Real ACPI cache leak point is as follows: [ 0.360101] ACPI: Added _OSI(Module Device) [ 0.360101] ACPI: Added _OSI(Processor Device) [ 0.360101] ACPI: Added _OSI(3.0 _SCP Extensions) [ 0.361043] ACPI: Added _OSI(Processor Aggregator Device) [ 0.364016] ACPI: Unable to start the ACPI Interpreter [ 0.365061] ACPI Error: Could not remove SCI handler (20170303/evmisc-281) [ 0.368174] kmem_cache_destroy Acpi-Parse: Slab cache still has objects [ 0.369332] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 4.12.0-rc4-next-20170608+ #8 [ 0.371256] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 0.372000] Call Trace: [ 0.372000] ? dump_stack+0x5c/0x81 [ 0.372000] ? kmem_cache_destroy+0x1aa/0x1c0 [ 0.372000] ? acpi_sleep_proc_init+0x27/0x27 [ 0.372000] ? acpi_os_delete_cache+0xa/0x10 [ 0.372000] ? acpi_ut_delete_caches+0x56/0x7b [ 0.372000] ? acpi_terminate+0xa/0x14 [ 0.372000] ? acpi_init+0x2af/0x34f [ 0.372000] ? __class_create+0x4c/0x80 [ 0.372000] ? video_setup+0x7f/0x7f [ 0.372000] ? acpi_sleep_proc_init+0x27/0x27 [ 0.372000] ? do_one_initcall+0x4e/0x1a0 [ 0.372000] ? kernel_init_freeable+0x189/0x20a [ 0.372000] ? rest_init+0xc0/0xc0 [ 0.372000] ? kernel_init+0xa/0x100 [ 0.372000] ? ret_from_fork+0x25/0x30 [ 0.388039] kmem_cache_destroy Acpi-ParseExt: Slab cache still has objects [ 0.389063] CPU: 1 PID: 1 Comm: swapper/0 Tainted: G W 4.12.0-rc4-next-20170608+ #8 [ 0.390557] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 [ 0.392000] Call Trace: [ 0.392000] ? dump_stack+0x5c/0x81 [ 0.392000] ? kmem_cache_destroy+0x1aa/0x1c0 [ 0.392000] ? acpi_sleep_proc_init+0x27/0x27 [ 0.392000] ? acpi_os_delete_cache+0xa/0x10 [ 0.392000] ? acpi_ut_delete_caches+0x6d/0x7b [ 0.392000] ? acpi_terminate+0xa/0x14 [ 0.392000] ? acpi_init+0x2af/0x34f [ 0.392000] ? __class_create+0x4c/0x80 [ 0.392000] ? video_setup+0x7f/0x7f [ 0.392000] ? acpi_sleep_proc_init+0x27/0x27 [ 0.392000] ? do_one_initcall+0x4e/0x1a0 [ 0.392000] ? kernel_init_freeable+0x189/0x20a [ 0.392000] ? rest_init+0xc0/0xc0 [ 0.392000] ? kernel_init+0xa/0x100 [ 0.392000] ? 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_ut_delete_caches() function to delete local caches (acpi_gbl_namespace_ cache, state_cache, operand_cache, ps_node_cache, ps_node_ext_cache). But the deletion codes in acpi_ut_delete_caches() function only delete slab caches using kmem_cache_destroy() function, therefore the cache objects should be flushed before acpi_ut_delete_caches() function. "Acpi-Parse" cache and "Acpi-ParseExt" cache are used in an AML parse function, acpi_ps_parse_loop(). The function should complete all ops using acpi_ps_complete_final_op() when an error occurs due to invalid AML codes. However, the current implementation of acpi_ps_complete_final_op() does not complete all ops when it meets some errors and this cause cache leak. This cache leak has a security threat because an old kernel (<= 4.9) shows memory locations of kernel functions in stack dump. Some malicious users could use this information to neutralize kernel ASLR. To fix ACPI cache leak for enhancing security, I made a patch to complete all ops unconditionally for acpi_ps_complete_final_op() function. I hope that this patch improves the security of Linux kernel. Thank you. Signed-off-by: Seunghun Han <kkamagui@gmail.com>
diff --git a/source/components/parser/psobject.c b/source/components/parser/psobject.c
@@ -768,7 +768,8 @@ AcpiPsCompleteFinalOp (
     ACPI_PARSE_OBJECT       *Op,
     ACPI_STATUS             Status)
 {
-    ACPI_STATUS             Status2;
+    ACPI_STATUS             ReturnStatus = Status;
+    BOOLEAN                 Ascending = TRUE;
 
 
     ACPI_FUNCTION_TRACE_PTR (PsCompleteFinalOp, WalkState);
@@ -785,7 +786,7 @@ AcpiPsCompleteFinalOp (
     {
         if (Op)
         {
-            if (WalkState->AscendingCallback != NULL)
+            if (Ascending && WalkState->AscendingCallback != NULL)
             {
                 WalkState->Op = Op;
                 WalkState->OpInfo = AcpiPsGetOpcodeInfo (Op->Common.AmlOpcode);
@@ -804,41 +805,28 @@ AcpiPsCompleteFinalOp (
 
                 if (Status == AE_CTRL_TERMINATE)
                 {
-                    Status = AE_OK;
-
-                    /* Clean up */
-                    do
-                    {
-                        if (Op)
-                        {
-                            Status2 = AcpiPsCompleteThisOp (WalkState, Op);
-                            if (ACPI_FAILURE (Status2))
-                            {
-                                return_ACPI_STATUS (Status2);
-                            }
-                        }
-
-                        AcpiPsPopScope (&(WalkState->ParserState), &Op,
-                            &WalkState->ArgTypes, &WalkState->ArgCount);
-
-                    } while (Op);
-
-                    return_ACPI_STATUS (Status);
+                    Ascending = FALSE;
+                    ReturnStatus = AE_CTRL_TERMINATE;
                 }
 
                 else if (ACPI_FAILURE (Status))
                 {
                     /* First error is most important */
 
-                    (void) AcpiPsCompleteThisOp (WalkState, Op);
-                    return_ACPI_STATUS (Status);
+                    Ascending = FALSE;
+                    ReturnStatus = Status;
                 }
             }
 
-            Status2 = AcpiPsCompleteThisOp (WalkState, Op);
-            if (ACPI_FAILURE (Status2))
+            Status = AcpiPsCompleteThisOp (WalkState, Op);
+            if (ACPI_FAILURE (Status))
             {
-                return_ACPI_STATUS (Status2);
+                Ascending = FALSE;
+                if (ACPI_SUCCESS (ReturnStatus) ||
+                    ReturnStatus == AE_CTRL_TERMINATE)
+                {
+                    ReturnStatus = Status;
+                }
             }
         }
 
@@ -847,5 +835,5 @@ AcpiPsCompleteFinalOp (
 
     } while (Op);
 
-    return_ACPI_STATUS (Status);
+    return_ACPI_STATUS (ReturnStatus);
 }

Original file line number	Diff line number	Diff line change
`@@ -768,7 +768,8 @@ AcpiPsCompleteFinalOp (`
`768`	`768`	`ACPI_PARSE_OBJECT *Op,`
`769`	`769`	`ACPI_STATUS Status)`
`770`	`770`	`{`
`771`		`- ACPI_STATUS Status2;`
	`771`	`+ ACPI_STATUS ReturnStatus = Status;`
	`772`	`+ BOOLEAN Ascending = TRUE;`
`772`	`773`
`773`	`774`
`774`	`775`	`ACPI_FUNCTION_TRACE_PTR (PsCompleteFinalOp, WalkState);`
`@@ -785,7 +786,7 @@ AcpiPsCompleteFinalOp (`
`785`	`786`	`{`
`786`	`787`	`if (Op)`
`787`	`788`	`{`
`788`		`- if (WalkState->AscendingCallback != NULL)`
	`789`	`+ if (Ascending && WalkState->AscendingCallback != NULL)`
`789`	`790`	`{`
`790`	`791`	`WalkState->Op = Op;`
`791`	`792`	`WalkState->OpInfo = AcpiPsGetOpcodeInfo (Op->Common.AmlOpcode);`
`@@ -804,41 +805,28 @@ AcpiPsCompleteFinalOp (`
`804`	`805`
`805`	`806`	`if (Status == AE_CTRL_TERMINATE)`
`806`	`807`	`{`
`807`		`- Status = AE_OK;`
`808`		`-`
`809`		`- /* Clean up */`
`810`		`- do`
`811`		`- {`
`812`		`- if (Op)`
`813`		`- {`
`814`		`- Status2 = AcpiPsCompleteThisOp (WalkState, Op);`
`815`		`- if (ACPI_FAILURE (Status2))`
`816`		`- {`
`817`		`- return_ACPI_STATUS (Status2);`
`818`		`- }`
`819`		`- }`
`820`		`-`
`821`		`- AcpiPsPopScope (&(WalkState->ParserState), &Op,`
`822`		`- &WalkState->ArgTypes, &WalkState->ArgCount);`
`823`		`-`
`824`		`- } while (Op);`
`825`		`-`
`826`		`- return_ACPI_STATUS (Status);`
	`808`	`+ Ascending = FALSE;`
	`809`	`+ ReturnStatus = AE_CTRL_TERMINATE;`
`827`	`810`	`}`
`828`	`811`
`829`	`812`	`else if (ACPI_FAILURE (Status))`
`830`	`813`	`{`
`831`	`814`	`/* First error is most important */`
`832`	`815`
`833`		`- (void) AcpiPsCompleteThisOp (WalkState, Op);`
`834`		`- return_ACPI_STATUS (Status);`
	`816`	`+ Ascending = FALSE;`
	`817`	`+ ReturnStatus = Status;`
`835`	`818`	`}`
`836`	`819`	`}`
`837`	`820`
`838`		`- Status2 = AcpiPsCompleteThisOp (WalkState, Op);`
`839`		`- if (ACPI_FAILURE (Status2))`
	`821`	`+ Status = AcpiPsCompleteThisOp (WalkState, Op);`
	`822`	`+ if (ACPI_FAILURE (Status))`
`840`	`823`	`{`
`841`		`- return_ACPI_STATUS (Status2);`
	`824`	`+ Ascending = FALSE;`
	`825`	`+ if (ACPI_SUCCESS (ReturnStatus) \|\|`
	`826`	`+ ReturnStatus == AE_CTRL_TERMINATE)`
	`827`	`+ {`
	`828`	`+ ReturnStatus = Status;`
	`829`	`+ }`
`842`	`830`	`}`
`843`	`831`	`}`
`844`	`832`
`@@ -847,5 +835,5 @@ AcpiPsCompleteFinalOp (`
`847`	`835`
`848`	`836`	`} while (Op);`
`849`	`837`
`850`		`- return_ACPI_STATUS (Status);`
	`838`	`+ return_ACPI_STATUS (ReturnStatus);`
`851`	`839`	`}`