Re: [Devel] [PATCH] ACPICA: Detect duplicate SSDT tables
by Moore, Robert
Does the machine or machines work properly with Windows? This is always one of our early questions.
Bob
> -----Original Message-----
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Sent: Tuesday, February 28, 2017 6:32 AM
> To: Zheng, Lv <lv.zheng(a)intel.com>; Rafael J . Wysocki
> <rjw(a)rjwysocki.net>; Len Brown <lenb(a)kernel.org>; Moore, Robert
> <robert.moore(a)intel.com>
> Cc: linux-acpi(a)vger.kernel.org; devel(a)acpica.org
> Subject: Re: [PATCH] ACPICA: Detect duplicate SSDT tables
>
> Hi,
>
> On 28-02-17 06:19, Zheng, Lv wrote:
> > Hi,
> >
> >> From: Hans de Goede [mailto:hdegoede@redhat.com]
> >> Subject: [PATCH] ACPICA: Detect duplicate SSDT tables
> >>
> >> Some machines have the exact (byte for byte) same SSDT tables
> >> multiple times in the root_table_list.
> >
> > Could you give a machine list here?
>
> Currently I'm seeing this on a GPD win machine:
>
> http://www.gpd.hk/gpdwin.asp
>
> I thought I was seeing it on more machines, but those have different
> apci table loading errors...
>
> >> Detect this and silently skip the duplicates rather then printing a
> >> scary looking set of errors.
> >
> > Why will this matter to OSPMs?
>
> Not sure what you mean with OSPMs but I can tell you why this matters in
> general, Linux distributions like e.g. Fedora have been putting a lot of
> work in a smooth boot experience where end users do not get any scary
> text messages. For some more embedded like systems this even is a hard
> requirement.
>
> The kernel supports quiet kernel cmdline argument to silence normal
> kernel messages, which is part of what is needed but messages with a log
> level of error still get shown, breaking the "no scary text messages"
> product requirement.
>
> > And should we add non-costless steps just in order to reduce errors,
>
> Yes we should, work on that front has been happening for years, also the
> CPU cost of this check is quite small, memcmp will only happen on
> identically sized tables and even then it will exit as soon as a single
> byte differs.
>
> > while the errors are on the contrary useful (in1dicating platform
> issues)?
>
> These errors are useful for developers / during testing but not in
> production setups, esp. in the case of duplicate tables where not
> loading the duplicate leads to 0 bad side effects.
>
> I've an alternative proposal though, since this check just fixes a small
> part of the early boot messages caused by SSDT loading and since the
> code itself chooses to ignore any errors:
>
> /* Ignore errors while loading tables, get as many as possible
> */
>
> How about setting a global flag while loading these tables and making
>
> ACPI_EXCEPTION calls log the exceptions with a log level of warning as
> well as turning the final:
>
> ACPI_ERROR((AE_INFO,
> "%u table load failures, %u successful",
> tables_failed, tables_loaded));
>
> Into a warning ?
>
> Regards,
>
> Hans
>
>
>
>
> >
> > Thanks
> > Lv
> >
> >>
> >> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
> >> ---
> >> drivers/acpi/acpica/tbxfload.c | 41
> >> ++++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 40 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/acpi/acpica/tbxfload.c
> >> b/drivers/acpi/acpica/tbxfload.c index 82019c0..1971cd7 100644
> >> --- a/drivers/acpi/acpica/tbxfload.c
> >> +++ b/drivers/acpi/acpica/tbxfload.c
> >> @@ -125,6 +125,44 @@ ACPI_EXPORT_SYMBOL_INIT(acpi_load_tables)
> >>
> >>
> /***********************************************************************
> ********
> >> *
> >> + * FUNCTION: acpi_tb_find_duplicate_ssdt
> >> + *
> >> + * PARAMETERS: table - validated acpi_table_desc of table
> to check
> >> + * index - index of table to find a duplicate
> of
> >> + *
> >> + * RETURN: TRUE if a duplicate is found, FALSE if not
> >> + *
> >> + * DESCRIPTION: Private helper function for acpi_tb_load_namespace
> to
> >> + * avoid trying to load duplicate ssdt tables
> >> + *
> >> +
> >> +********************************************************************
> >> +**********/ static u8 acpi_tb_find_duplicate_ssdt(struct
> >> +acpi_table_desc *table, u32 index) {
> >> + struct acpi_table_desc *dup;
> >> + u32 i;
> >> +
> >> + for (i = 0; i < index; ++i) {
> >> + dup = &acpi_gbl_root_table_list.tables[i];
> >> +
> >> + if (!acpi_gbl_root_table_list.tables[i].address ||
> >> + (!ACPI_COMPARE_NAME(dup->signature.ascii, ACPI_SIG_SSDT)
> >> + && !ACPI_COMPARE_NAME(dup->signature.ascii,
> >> + ACPI_SIG_PSDT)
> >> + && !ACPI_COMPARE_NAME(dup->signature.ascii,
> >> + ACPI_SIG_OSDT))
> >> + || ACPI_FAILURE(acpi_tb_validate_table(dup))
> >> + || dup->length != table->length) {
> >> + continue;
> >> + }
> >> +
> >> + if (memcmp(dup->pointer, table->pointer, table->length) == 0)
> >> + return TRUE;
> >> + }
> >> + return FALSE;
> >> +}
> >> +
> >> +/*******************************************************************
> >> +************
> >> + *
> >> * FUNCTION: acpi_tb_load_namespace
> >> *
> >> * PARAMETERS: None
> >> @@ -212,7 +250,8 @@ acpi_status acpi_tb_load_namespace(void)
> >> ACPI_SIG_PSDT)
> >> && !ACPI_COMPARE_NAME(table->signature.ascii,
> >> ACPI_SIG_OSDT))
> >> - || ACPI_FAILURE(acpi_tb_validate_table(table))) {
> >> + || ACPI_FAILURE(acpi_tb_validate_table(table))
> >> + || acpi_tb_find_duplicate_ssdt(table, i)) {
> >> continue;
> >> }
> >>
> >> --
> >> 2.9.3
> >
4 years, 5 months
- 1
- 0
Re: [Devel] [PATCH] ACPICA: Detect duplicate SSDT tables
by Zheng, Lv
Hi,
> From: Hans de Goede [mailto:hdegoede@redhat.com]
> Subject: [PATCH] ACPICA: Detect duplicate SSDT tables
>
> Some machines have the exact (byte for byte) same SSDT tables multiple
> times in the root_table_list.
Could you give a machine list here?
> Detect this and silently skip the duplicates
> rather then printing a scary looking set of errors.
Why will this matter to OSPMs?
And should we add non-costless steps just in order to reduce errors,
while the errors are on the contrary useful (indicating platform issues)?
Thanks
Lv
>
> Signed-off-by: Hans de Goede <hdegoede(a)redhat.com>
> ---
> drivers/acpi/acpica/tbxfload.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> index 82019c0..1971cd7 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -125,6 +125,44 @@ ACPI_EXPORT_SYMBOL_INIT(acpi_load_tables)
>
> /*******************************************************************************
> *
> + * FUNCTION: acpi_tb_find_duplicate_ssdt
> + *
> + * PARAMETERS: table - validated acpi_table_desc of table to check
> + * index - index of table to find a duplicate of
> + *
> + * RETURN: TRUE if a duplicate is found, FALSE if not
> + *
> + * DESCRIPTION: Private helper function for acpi_tb_load_namespace to
> + * avoid trying to load duplicate ssdt tables
> + *
> + ******************************************************************************/
> +static u8 acpi_tb_find_duplicate_ssdt(struct acpi_table_desc *table, u32 index)
> +{
> + struct acpi_table_desc *dup;
> + u32 i;
> +
> + for (i = 0; i < index; ++i) {
> + dup = &acpi_gbl_root_table_list.tables[i];
> +
> + if (!acpi_gbl_root_table_list.tables[i].address ||
> + (!ACPI_COMPARE_NAME(dup->signature.ascii, ACPI_SIG_SSDT)
> + && !ACPI_COMPARE_NAME(dup->signature.ascii,
> + ACPI_SIG_PSDT)
> + && !ACPI_COMPARE_NAME(dup->signature.ascii,
> + ACPI_SIG_OSDT))
> + || ACPI_FAILURE(acpi_tb_validate_table(dup))
> + || dup->length != table->length) {
> + continue;
> + }
> +
> + if (memcmp(dup->pointer, table->pointer, table->length) == 0)
> + return TRUE;
> + }
> + return FALSE;
> +}
> +
> +/*******************************************************************************
> + *
> * FUNCTION: acpi_tb_load_namespace
> *
> * PARAMETERS: None
> @@ -212,7 +250,8 @@ acpi_status acpi_tb_load_namespace(void)
> ACPI_SIG_PSDT)
> && !ACPI_COMPARE_NAME(table->signature.ascii,
> ACPI_SIG_OSDT))
> - || ACPI_FAILURE(acpi_tb_validate_table(table))) {
> + || ACPI_FAILURE(acpi_tb_validate_table(table))
> + || acpi_tb_find_duplicate_ssdt(table, i)) {
> continue;
> }
>
> --
> 2.9.3
4 years, 5 months
- 1
- 0
Re: [Devel] [PATCH v2] acpi: acpica: fix acpi operand cache leak
by Zheng, Lv
Hi, Rafael
> From: linux-acpi-owner(a)vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J.
> Wysocki
> Subject: Re: [PATCH v2] acpi: acpica: fix acpi operand cache leak
>
> On Fri, Feb 24, 2017 at 11:37 PM, Seunghun Han <kkamagui(a)gmail.com> wrote:
> > Hi, Rafael.
> >
> > I agree with you and I added my opinion below.
> >
> > 2017-02-25 1:50 GMT+09:00 Rafael J. Wysocki <rjw(a)rjwysocki.net>:
> >> On Friday, February 24, 2017 09:56:21 PM Seunghun Han wrote:
> >>> Hi, Rafeal.
> >>>
> >>> I added my opinion below.
> >>>
> >>> 2017-02-24 21:13 GMT+09:00 Rafael J. Wysocki <rjw(a)rjwysocki.net>:
> >>> > On Friday, February 24, 2017 09:15:52 PM Seunghun Han wrote:
> >>> >> Hi, Rafael.
> >>> >>
> >>> >> I added my opinion below.
> >>> >>
> >>> >> 2017-02-24 20:50 GMT+09:00 Rafael J. Wysocki <rjw(a)rjwysocki.net>:
> >>> >> > 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 <kkamagui(a)gmail.com>:
> >>> >> >> > Hello,
> >>> >> >> >
> >>> >> >> > I attached the test results below,
> >>> >> >> >
> >>> >> >> > 2017-02-21 9:53 GMT+09:00 Rowafael J. Wysocki <rjw(a)rjwysocki.net>:
> >>> >> >> >> On Tuesday, February 21, 2017 12:33:08 AM Zheng, Lv wrote:
> >>> >> >> >>> Hi,
> >>> >> >> >>>
> >>> >> >> >>> > From: linux-acpi-owner(a)vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On
> Behalf Of Seunghun
> >>> >> >> >>> > Han
> >>> >> >> >>> > Subject: [PATCH v2] acpi: acpica: fix acpi operand cache leak
> >>> >> >> >>> >
> >>> >> >> >>> > 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
> >>> >> >> >>>
> >>> >> >> >>> 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
> >>> >> >
> >>> >>
> >>> >> 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 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.
> >>
> >> 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.
> >
> > 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.
>
> 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?
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 majordomo(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
4 years, 5 months
- 1
- 0
ACPICA version 20170224 released
by Moore, Robert
24 February 2017. Summary of changes for version 20170224:
This release is available at https://acpica.org/downloads
1) ACPICA kernel-resident subsystem:
Interpreter: Fixed two issues with the control method return value auto-repair feature, where an attempt to double-delete an internal object could result in an ACPICA warning (for _CID repair and others). No fault occurs, however, because the attempted deletion (actually a release to an internal cache) is detected and ignored via object poisoning.
Debugger: Fixed an AML interpreter mutex issue during the single stepping of control methods. If certain debugger commands are executed during stepping, a mutex aquire/release error could occur. Lv Zheng.
Fixed some issues generating ACPICA with the Intel C compiler by restoring the original behavior and compiler-specific include file in acenv.h. Lv Zheng.
Example Code and Data Size: These are the sizes for the OS-independent acpica.lib produced by the Microsoft Visual C++ 9.0 32-bit compiler. The debug version of the code includes the debug output trace mechanism and has a much larger code and data size.
Current Release:
Non-Debug Version: 141.7K Code, 58.5K Data, 200.2K Total
Debug Version: 207.5K Code, 82.7K Data, 290.2K Total
Previous Release:
Non-Debug Version: 137.4K Code, 52.6K Data, 190.0K Total
Debug Version: 201.5K Code, 82.2K Data, 283.7K Total
2) iASL Compiler/Disassembler and Tools:
iASL/Disassembler: A preliminary version of a new ASL-to-ASL+ conversion tool has been designed, implemented, and included in this release. The key feature of this utility is that the original comments within the input ASL file are preserved during the conversion process, and included within the converted ASL+ file -- thus creating a transparent conversion of existing ASL files to ASL+ (ASL 2.0). Erik Schmauss.
Usage: iasl -ca <ASL-filename> // Output is a .dsl file with converted code
iASL/Disassembler: Improved the detection and correct disassembly of Switch/Case operators. This feature detects sequences of if/elseif/else operators that originated from ASL Switch/Case/Default operators and emits the original operators. David Box.
iASL: Improved the IORT ACPI table support in the following areas. Lv Zheng:
Clear MappingOffset if the MappingCount is zero.
Fix the disassembly of the SMMU GSU interrupt offset.
Update the template file for the IORT table.
Disassembler: Enhanced the detection and disassembly of resource template/descriptor within a Buffer object. An EndTag descriptor is now required to have a zero second byte, since all known ASL compilers emit this. This helps eliminate incorrect decisions when a buffer is disassembled (false positives on resource templates).
4 years, 5 months
- 1
- 0
Re: [Devel] Devel Digest, Vol 72, Issue 1: ACPICA version 20161117 released (Moore, Robert)
by Alexei Fedorov
Hi,
When ACPI IORT template is created with "iasl.exe -T IORT", the generated ITS group node
also contains 1 entry in the array of ID mappings:
[0001] Type : 00
[0002] Length : 002C
[0001] Revision : 00
[0004] Reserved : 00000000
[0004] Mapping Count : 00000001
[0004] Mapping Offset : 00000018
[0004] ItsCount : 00000001
[0004] Identifiers : 00000000
[0004] Input base : 00000000
[0004] ID Count : 00000000
[0004] Output Base : 00000000
[0004] Output Reference : 00000000
[0004] Flags (decoded below) : 00000000
Single Mapping : 0
This contradicts with the IO Remapping Table specification
(http://infocenter.arm.com/help/topic/com.arm.doc.den0049b/DEN0049B_IO_Rem...) Table 11 which reads:
"Number of ID mappings: This field has a value of 0. ITS groups do not have IDs.
Reference to ID Array: This field has a value of 0. There is no ID array."
That implies that "Mapping Count" and "Mapping Offset" fields should be set to 0 and ID array
data removed.
If the Iort.asl template is compiled and then Iort.aml binary disassembled to Iort.dsl,
SMMUv1 node lists incorrect offset for SMMU_NSgCfgIrpt Interrupt which has the same [0ECh 0236] value as for SMMU_NSgIrpt:
[0ECh 0236 8] SMMU_NSgIrpt Interrupt : 0000000000000000
[0ECh 0236 8] SMMU_NSgCfgIrpt Interrupt : 0000000000000000
but should be set to [0F4h 0244]
Alexei.
-----Original Message-----
From: Devel [mailto:devel-bounces@acpica.org] On Behalf Of devel-request(a)acpica.org
Sent: 19 December 2016 06:05
To: devel(a)acpica.org
Subject: Devel Digest, Vol 72, Issue 1
Send Devel mailing list submissions to
devel(a)acpica.org
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.acpica.org/mailman/listinfo/devel
or, via email, send a message with subject or body 'help' to
devel-request(a)acpica.org
You can reach the person managing the list at
devel-owner(a)acpica.org
When replying, please edit your Subject line so it is more specific than "Re: Contents of Devel digest..."
Today's Topics:
1. Re: [PATCH] ACPI: allow compilation with bare metal compilers
(Al Stone)
2. patch request for tbxfload.c (scole_mail)
3. ACPICA version 20161117 released (Moore, Robert)
4. Re: Question about the code in
source/components/hardware/hwxfsleep.c (Al Stone)
5. [PATCH] Linux-specific header: Add support for s390x
compilation (Colin King)
6. [PATCH] Linux-specific header: Add support for s390x
compilation. (Colin King)
7. Re: [PATCH] acpi: Fix format string type mistakes (Zheng, Lv)
----------------------------------------------------------------------
Message: 1
Date: Tue, 15 Nov 2016 09:36:05 -0700
From: Al Stone <ahs3(a)redhat.com>
To: "Moore, Robert" <robert.moore(a)intel.com>,
"linux-acpi(a)vger.kernel.org" <linux-acpi(a)vger.kernel.org>,
"devel(a)acpica.org" <devel(a)acpica.org>, "linux-kernel(a)vger.kernel.org"
<linux-kernel(a)vger.kernel.org>
Cc: "Rafael J . Wysocki" <rjw(a)rjwysocki.net>, Len Brown
<lenb(a)kernel.org>, "Zheng, Lv" <lv.zheng(a)intel.com>
Subject: Re: [Devel] [PATCH] ACPI: allow compilation with bare metal
compilers
Message-ID: <ac74e311-8482-9336-83c7-0eb5e84d81a5(a)redhat.com>
Content-Type: text/plain; charset=windows-1252
On 11/15/2016 08:43 AM, Moore, Robert wrote:
> The design for all of this is as follows:
>
> 1) OS-dependent includes
> 2) Compiler-specific includes
> 3) acenv.h is the master file that pulls in the correct headers (one
> compiler, and one OS)
Sure, that's understood from the current structure. The issue is #3 -- there is no OS, in the sense of a run-time environment, so that assumption is incorrect in this case. All we know is that we're compiling the kernel and it needs to be able to run in complete isolation.
> So, I think I see a couple of possible solutions for you:
>
> 1) If you are using GCC, the __GNUC__ symbol should already be defined.
Right, which is what this patch is relying on.
> 2) If "aclinux.h" works for you, we can either add a conditional case that would apply to your environment, or:
I'm confused; I thought that was what this patch was doing. What is being suggested instead?
> 2a) You could define _LINUX in your gcc invocations.
This option might force changes in the kernel build which is otherwise not affected; this is only an issue with drivers/acpi/acpica/*. It may also cause ACPICA to build incorrectly for other OSs, but I'll have to look at that. Off hand, it seems like this would cause more problems with acenv.h.
>
> Because ACPICA supports many different environments, we don't want to have a "default" case which in a sense would only be an attempt to guess what the user intended. We want to have a clear error that tells the user that something important needs to be done before the code can be compiled.
Understood, which is why I left the #error case; the intent is only to have a default when compiling Linux for bare-metal environments using GCC. Perhaps this is just a Linux-only modification...
> Bob
>
>
>> -----Original Message-----
>> From: Al Stone [mailto:ahs3@redhat.com]
>> Sent: Monday, November 14, 2016 3:09 PM
>> To: linux-acpi(a)vger.kernel.org; devel(a)acpica.org; linux-
>> kernel(a)vger.kernel.org
>> Cc: Al Stone <ahs3(a)redhat.com>; Rafael J . Wysocki
>> <rjw(a)rjwysocki.net>; Len Brown <lenb(a)kernel.org>; Moore, Robert
>> <robert.moore(a)intel.com>; Zheng, Lv <lv.zheng(a)intel.com>
>> Subject: [PATCH] ACPI: allow compilation with bare metal compilers
>>
>> The ACPICA subsystem of the ACPI driver sets up a compilation
>> environment for itself, adding in multiple typedefs unique to ACPICA
>> that depend on where ACPICA will be used.
>>
>> The vast majority of such environments (Linux, QNX, ...) have an
>> environment defined by the acenv.h header file. When using a Linaro
>> compiler [1] specifically built to be used in an embedded environment
>> with perhaps a kernel and an init process as the only things running,
>> there is no environment defined for ACPICA so the typedefs it needs
>> are not set up, causing compilation to fail badly unless ACPI is
>> completely disabled.
>> Since ACPI is enabled in the default config for the kernel, the
>> compilation failure is fairly obvious.
>>
>> This may not be the optimal solution, but add in to the ACPI header
>> file include/acpi/platform/acenv.h a default so that if GCC is being
>> used, and all else fails, assume that we are going to be in a
>> Linux-like environment and re-use the environment definition for
>> Linux. This allows us to build a kernel using this compiler [1] with
>> or without ACPI.
>>
>> [1]
>> https://releases.linaro.org/components/toolchain/binaries/latest/aarc
>> h64 -elff/gcc-linaro-6.1.1-2016-08-x86_64_aarch64-elf.tar.xz
>>
>> Signed-off-by: Al Stone <ahs3(a)redhat.com>
>> Cc: Rafael J. Wysocki <rjw(a)rjwysocki.net>
>> Cc: Len Brown <lenb(a)kernel.org>
>> Cc: Robert Moore <robert.moore(a)intel.com>
>> Cc: Lv Zheng <lv.zheng(a)intel.com>
>> ---
>> include/acpi/platform/acenv.h | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/include/acpi/platform/acenv.h
>> b/include/acpi/platform/acenv.h index 34cce72..cdd1cd6 100644
>> --- a/include/acpi/platform/acenv.h
>> +++ b/include/acpi/platform/acenv.h
>> @@ -234,6 +234,21 @@
>> #elif defined(_AED_EFI) || defined(_GNU_EFI) || defined(_EDK2_EFI)
>> #include "acefi.h"
>>
>> +/*
>> + * Up to this point, we've been looking for specific environments.
>> +In
>> + * some cases, there is no environment, and we're just working on
>> +bare
>> + * metal. However, since we're compiling the Linux kernel, let's
>> +just
>> + * pretend we're in a Linux environment.
>> + */
>> +#elif defined(__GNUC__) && !defined(__INTEL_COMPILER) #if
>> +!defined(_LINUX) #define _LINUX #endif #if !defined(__linux__)
>> +#define __linux__ #endif #include <acpi/platform/aclinux.h>
>> +
>> #else
>>
>> /* Unknown environment */
>> --
>> 2.10.2
>
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------
------------------------------
Message: 2
Date: Mon, 14 Nov 2016 08:58:57 -0800
From: scole_mail <scole_mail(a)gmx.com>
To: devel(a)acpica.org
Subject: [Devel] patch request for tbxfload.c
Message-ID: <87poly0zhq.fsf(a)gmx.com>
Content-Type: text/plain
I was hoping this patch could be added. I am trying to run
netbsd/ia64 on a simulator (ski). Without the patch, I get a
segfault. With it, I get the same messages as previous acpica
versions with no seg-faulting.
I'm not sure if it is a just a ia64/ski issue, but it seems like
AcpiGbl_DsdtIndex is never getting set. It looks like only
AcpiTbInstallFixedTable() sets it, but I'm not seeing that called.
AcpiTbInstallTableWithOverride() has a comment saying it sets it, but
I don't see AcpiGbl_DsdtIndex passed to it either.
Output with the patch is below.
Thanks
*** ./acpica-unix-20160930/source/components/tables/tbxfload.c.origMon Nov 14 08:44:43 2016
--- ./acpica-unix-20160930/source/components/tables/tbxfload.cMon Nov 14 08:47:04 2016
***************
*** 236,241 ****
--- 236,250 ----
(void) AcpiUtAcquireMutex (ACPI_MTX_TABLES);
/*
+ * If AcpiGbl_DsdtIndex uninitialized, give up.
+ */
+ if (AcpiGbl_DsdtIndex == ACPI_INVALID_TABLE_INDEX)
+ {
+ Status = AE_NO_ACPI_TABLES;
+ goto UnlockAndExit;
+ }
+
+ /*
* Load the namespace. The DSDT is required, but any SSDT and
* PSDT tables are optional. Verify the DSDT.
*/
ACPI: RSDP 0x0000000000120420 00002C (v02 FBSD )
ACPI: XSDT 0x0000000000120444 00002C (v00 FBSD SKI 00000000 FBSD 00000000)
ACPI BIOS Warning (bug): Incorrect checksum in table [APIC] - 0x00, should be 0x0C (20160930/tbprint-233)
ACPI: APIC 0x0000000000120470 000080 (v00 FBSD SKI 00000000 FBSD 00000000)
ACPI Exception: AE_NO_ACPI_TABLES, While loading namespace from ACPI tables (20160930/tbxfload-111)
ACPI Warning: AcpiEnable failed (20160930/utxfinit-184)
acpi_probe: failed to enable subsystem
ACPI Error: Could not remove SCI handler (20160930/evmisc-312)
acpi0 at mainbus0: Intel ACPICA 20160930
acpi0: X/RSDT: OemId < FBSD, SKI,00000000>, AslId <FBSD,00000000>
ACPI Warning: AcpiEnable failed (20160930/utxfinit-184)
acpi_attach: failed to initialize ACPI: AE_NO_ACPI_TABLES
------------------------------
Message: 3
Date: Thu, 17 Nov 2016 17:42:39 +0000
From: "Moore, Robert" <robert.moore(a)intel.com>
To: acpica.org list <devel(a)acpica.org>
Subject: [Devel] ACPICA version 20161117 released
Message-ID:
<94F2FBAB4432B54E8AACC7DFDE6C92E37E52E0B6(a)ORSMSX110.amr.corp.intel.com>
Content-Type: text/plain; charset="us-ascii"
17 November 2016. Summary of changes for version 20161117:
This release is available at https://acpica.org/downloads
1) ACPICA kernel-resident subsystem:
Table Manager: Fixed a regression introduced in 20160729, "FADT support cleanup". This was an attempt to remove all references in the source to the FADT version 2, which never was a legal version number. It was skipped because it was an early version of 64-bit support that was eventually abandoned for the current 64-bit support.
Interpreter: Fixed a problem where runtime implicit conversion was incorrectly disabled for the ASL operators below. This brings the behavior into compliance with the ACPI specification:
FromBCD
ToBCD
ToDecimalString
ToHexString
ToInteger
ToBuffer
Table Manager: Added a new public interface, AcpiPutTable, used to release and free an ACPI table returned by AcpiGetTable and related interfaces. Lv Zheng.
Example Code and Data Size: These are the sizes for the OS-independent acpica.lib produced by the Microsoft Visual C++ 9.0 32-bit compiler. The debug version of the code includes the debug output trace mechanism and has a much larger code and data size.
Current Release:
Non-Debug Version: 140.5K Code, 58.5K Data, 198.9K Total
Debug Version: 201.3K Code, 82.7K Data, 284.0K Total
Previous Release:
Non-Debug Version: 140.4K Code, 58.1K Data, 198.5K Total
Debug Version: 200.7K Code, 82.1K Data, 282.8K Total
2) iASL Compiler/Disassembler and Tools:
Disassembler: Fixed a regression for disassembly of Resource Template. Detection of templates in the AML stream missed some types of templates.
iASL: Fixed a problem where an Access Size error was returned for the PCC address space when the AccessSize of the GAS register is greater than a DWORD. Hoan Tran.
iASL: Implemented several grammar changes for the operators below. These changes are slated for the next version of the ACPI specification:
RefOf - Disallow method invocation as an operand
CondRefOf - Disallow method invocation as an operand
DerefOf - Disallow operands that use the result from operators that
do not return a reference (Changed TermArg to SuperName).
iASL: Control method invocations are now allowed for Target operands, as per the ACPI specification. Removed error for using a control method invocation as a Target operand.
Disassembler: Improved detection of Resource Templates, Unicode, and Strings within Buffer objects. These subtypes do not contain a specific opcode to indicate the originating ASL code, and they must be detected by other means within the disassembler.
iASL: Implemented an optimization improvement for 32-bit ACPI tables (DSDT/SSDT). For the 32-bit case only, compute the optimum integer opcode only after 64-bit to 32-bit truncation. A truncation warning message is still emitted, however.
AcpiXtract: Implemented handling for both types of line terminators (LF or CR/LF) so that it can accept AcpiDump output files from any system. Peter Wu.
AcpiBin: Added two new options for comparing AML files:
-a: compare and display ALL mismatches
-o: start compare at this offset into the second file
------------------------------
Message: 4
Date: Mon, 21 Nov 2016 12:46:52 -0700
From: Al Stone <ahs3(a)redhat.com>
To: Kein Yuan <kein.yuan(a)yahoo.com>, "devel(a)acpica.org"
<devel(a)acpica.org>
Subject: Re: [Devel] Question about the code in
source/components/hardware/hwxfsleep.c
Message-ID: <08d8e4d2-bc84-52bf-f8af-c7fbd469754f(a)redhat.com>
Content-Type: text/plain; charset=windows-1252
On 11/11/2016 04:53 PM, Kein Yuan wrote:
> Hi ACPICA experts,
> A quick question,
>
> File: source/components/hardware/hwxfsleep.c
> Function: AcpiHwSetFirmwareWakingVector
>
> static ACPI_STATUS
> AcpiHwSetFirmwareWakingVector (
> ACPI_TABLE_FACS *Facs,
> ACPI_PHYSICAL_ADDRESS PhysicalAddress,
> ACPI_PHYSICAL_ADDRESS PhysicalAddress64)
> {
> ACPI_FUNCTION_TRACE (AcpiHwSetFirmwareWakingVector);
>
>
> /*
> * According to the ACPI specification 2.0c and later, the 64-bit
> * waking vector should be cleared and the 32-bit waking vector should
> * be used, unless we want the wake-up code to be called by the BIOS in
> * Protected Mode. Some systems (for example HP dv5-1004nr) are known
> * to fail to resume if the 64-bit vector is used.
> */
>
> /* Set the 32-bit vector */
>
> Facs->FirmwareWakingVector = (UINT32) PhysicalAddress;
>
> if (Facs->Length > 32) // ---------------------> Question for this compare
> {
> if (Facs->Version >= 1)
> {
> /* Set the 64-bit vector */
>
> Facs->XFirmwareWakingVector = PhysicalAddress64;
> }
> else
> {
> /* Clear the 64-bit vector if it exists */
>
> Facs->XFirmwareWakingVector = 0;
> }
> }
>
> return_ACPI_STATUS (AE_OK);
> }
>
> Since the FACS structure defined as:
> typedef struct acpi_table_facs
> {
> char Signature[4]; /* ASCII table signature */
> UINT32 Length; /* Length of structure, in
> bytes */
> UINT32 HardwareSignature; /* Hardware configuration
> signature */
> UINT32 FirmwareWakingVector; /* 32-bit physical address
> of the Firmware Waking Vector */
> UINT32 GlobalLock; /* Global Lock for shared
> hardware resources */
> UINT32 Flags;
> UINT64 XFirmwareWakingVector; /* 64-bit version of the
> Firmware Waking Vector (ACPI 2.0+) */
> UINT8 Version; /* Version of this table
> (ACPI 2.0+) */
> UINT8 Reserved[3]; /* Reserved, must be zero */
> UINT32 OspmFlags; /* Flags to be set by OSPM
> (ACPI 4.0) */
> UINT8 Reserved1[24]; /* Reserved, must be zero */
>
> } ACPI_TABLE_FACS;
>
>
> May I know in what case Facs->Length < 32?
>
> If looks to me that Facs->XFirmwareWakingVector = PhysicalAddress64; will always
> be executed?
>
> Thanks,
> Kein
IIRC, very old versions of the FACS were < 32 bytes long; those old versions
did not have a XFirmwareWakingVector field, or a proper Version field. For
that case, the function just returns after doing nothing. If the FACS is
larger than 32 bytes, it should have a Version field, but it looks like version
1 does not have a usable XFirmwareWakingVector field -- i.e., it is defined in
the struct, but is not valid until a later version of the ACPI specification
where Facs->Version >= 2. So, for Version == 1, ignore the field, but for
versions > 1, set the field.
Does that help?
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3(a)redhat.com
-----------------------------------
------------------------------
Message: 5
Date: Wed, 30 Nov 2016 13:16:26 +0000
From: Colin King <colin.king(a)canonical.com>
To: devel(a)acpica.org
Subject: [Devel] [PATCH] Linux-specific header: Add support for s390x
compilation
Message-ID: <20161130131627.25030-1-colin.king(a)canonical.com>
Content-Type: text/plain; charset="utf-8"
From: Colin Ian King <colin.king(a)canonical.com>
While porting ACPCIA integrated into fwts I discovered that I get build
issues on s390x because ACPCIA does not know it is a 64 bit architecture.
Update aclinux.h to support this architecture.
Colin Ian King (1):
Linux-specific header: Add support for s390x compilation.
source/include/platform/aclinux.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
2.10.2
------------------------------
Message: 6
Date: Wed, 30 Nov 2016 13:16:27 +0000
From: Colin King <colin.king(a)canonical.com>
To: devel(a)acpica.org
Subject: [Devel] [PATCH] Linux-specific header: Add support for s390x
compilation.
Message-ID: <20161130131627.25030-2-colin.king(a)canonical.com>
Content-Type: text/plain; charset="utf-8"
From: Colin Ian King <colin.king(a)canonical.com>
Adds s390x as a 64-bit architecture.
Signed-off-by: Colin Ian King <colin.king(a)canonical.com>
---
source/include/platform/aclinux.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source/include/platform/aclinux.h b/source/include/platform/aclinux.h
index 6f7c577..9d0927b 100644
--- a/source/include/platform/aclinux.h
+++ b/source/include/platform/aclinux.h
@@ -265,7 +265,8 @@
#define ACPI_CAST_PTHREAD_T(Pthread) ((ACPI_THREAD_ID) (Pthread))
#if defined(__ia64__) || defined(__x86_64__) ||\
- defined(__aarch64__) || defined(__PPC64__)
+ defined(__aarch64__) || defined(__PPC64__) ||\
+ defined(__s390x__)
#define ACPI_MACHINE_WIDTH 64
#define COMPILER_DEPENDENT_INT64 long
#define COMPILER_DEPENDENT_UINT64 unsigned long
--
2.10.2
------------------------------
Message: 7
Date: Mon, 19 Dec 2016 06:05:09 +0000
From: "Zheng, Lv" <lv.zheng(a)intel.com>
To: Kees Cook <keescook(a)chromium.org>, "Moore, Robert"
<robert.moore(a)intel.com>
Cc: "linux-kernel(a)vger.kernel.org" <linux-kernel(a)vger.kernel.org>,
"Wysocki, Rafael J" <rafael.j.wysocki(a)intel.com>, Len Brown
<lenb(a)kernel.org>, "linux-acpi(a)vger.kernel.org"
<linux-acpi(a)vger.kernel.org>, Emese Revfy <re.emese(a)gmail.com>,
"devel(a)acpica.org" <devel(a)acpica.org>
Subject: Re: [Devel] [PATCH] acpi: Fix format string type mistakes
Message-ID:
<1AE640813FDE7649BE1B193DEA596E886CDFF331(a)SHSMSX101.ccr.corp.intel.com>
Content-Type: text/plain; charset="us-ascii"
Hi,
> From: Kees Cook [mailto:keescook@chromium.org]
> Subject: [PATCH] acpi: Fix format string type mistakes
>
> From: Emese Revfy <re.emese(a)gmail.com>
>
> This adds the missing __printf attribute which allows compile time
> format string checking (and will be used by the coming initify gcc
> plugin). Additionally, this fixes the warnings exposed by the attribute.
>
> Signed-off-by: Emese Revfy <re.emese(a)gmail.com>
> [kees: split scsi/acpi, merged attr and fix, new commit messages]
> Signed-off-by: Kees Cook <keescook(a)chromium.org>
> ---
> drivers/acpi/acpica/dbhistry.c | 2 +-
> drivers/acpi/acpica/dbinput.c | 10 ++---
> drivers/acpi/acpica/dbstats.c | 88 +++++++++++++++++++++---------------------
> drivers/acpi/acpica/utdebug.c | 2 +-
> include/acpi/acpiosxf.h | 3 +-
> 5 files changed, 53 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/acpi/acpica/dbhistry.c b/drivers/acpi/acpica/dbhistry.c
> index 46bd65d38df9..ec9da4830f6a 100644
> --- a/drivers/acpi/acpica/dbhistry.c
> +++ b/drivers/acpi/acpica/dbhistry.c
> @@ -155,7 +155,7 @@ void acpi_db_display_history(void)
>
> for (i = 0; i < acpi_gbl_num_history; i++) {
> if (acpi_gbl_history_buffer[history_index].command) {
> -acpi_os_printf("%3ld %s\n",
> +acpi_os_printf("%3u %s\n",
> acpi_gbl_history_buffer[history_index].
> cmd_num,
> acpi_gbl_history_buffer[history_index].
> diff --git a/drivers/acpi/acpica/dbinput.c b/drivers/acpi/acpica/dbinput.c
> index 068214f9cc9d..43be06bdb790 100644
> --- a/drivers/acpi/acpica/dbinput.c
> +++ b/drivers/acpi/acpica/dbinput.c
> @@ -608,7 +608,7 @@ static u32 acpi_db_get_line(char *input_buffer)
> (acpi_gbl_db_parsed_buf, sizeof(acpi_gbl_db_parsed_buf),
> input_buffer)) {
> acpi_os_printf
> - ("Buffer overflow while parsing input line (max %u characters)\n",
> + ("Buffer overflow while parsing input line (max %lu characters)\n",
> sizeof(acpi_gbl_db_parsed_buf));
> return (0);
> }
> @@ -864,24 +864,24 @@ acpi_db_command_dispatch(char *input_buffer,
>
> if (param_count == 0) {
> acpi_os_printf
> - ("Current debug level for file output is: %8.8lX\n",
> + ("Current debug level for file output is: %8.8X\n",
> acpi_gbl_db_debug_level);
> acpi_os_printf
> - ("Current debug level for console output is: %8.8lX\n",
> + ("Current debug level for console output is: %8.8X\n",
> acpi_gbl_db_console_debug_level);
> } else if (param_count == 2) {
> temp = acpi_gbl_db_console_debug_level;
> acpi_gbl_db_console_debug_level =
> strtoul(acpi_gbl_db_args[1], NULL, 16);
> acpi_os_printf
> - ("Debug Level for console output was %8.8lX, now %8.8lX\n",
> + ("Debug Level for console output was %8.8X, now %8.8X\n",
> temp, acpi_gbl_db_console_debug_level);
> } else {
> temp = acpi_gbl_db_debug_level;
> acpi_gbl_db_debug_level =
> strtoul(acpi_gbl_db_args[1], NULL, 16);
> acpi_os_printf
> - ("Debug Level for file output was %8.8lX, now %8.8lX\n",
> + ("Debug Level for file output was %8.8X, now %8.8X\n",
> temp, acpi_gbl_db_debug_level);
> }
> break;
> diff --git a/drivers/acpi/acpica/dbstats.c b/drivers/acpi/acpica/dbstats.c
> index a414e1fa6f9d..de7023024b12 100644
> --- a/drivers/acpi/acpica/dbstats.c
> +++ b/drivers/acpi/acpica/dbstats.c
> @@ -377,17 +377,17 @@ acpi_status acpi_db_display_statistics(char *type_arg)
> "ACPI_TYPE", "NODES", "OBJECTS");
>
> for (i = 0; i < ACPI_TYPE_NS_NODE_MAX; i++) {
> -acpi_os_printf("%16.16s % 10ld% 10ld\n",
> +acpi_os_printf("%16.16s % 10d% 10d\n",
> acpi_ut_get_type_name(i),
> acpi_gbl_node_type_count[i],
> acpi_gbl_obj_type_count[i]);
> }
>
> -acpi_os_printf("%16.16s % 10ld% 10ld\n", "Misc/Unknown",
> +acpi_os_printf("%16.16s % 10d% 10d\n", "Misc/Unknown",
> acpi_gbl_node_type_count_misc,
> acpi_gbl_obj_type_count_misc);
>
> -acpi_os_printf("%16.16s % 10ld% 10ld\n", "TOTALS:",
> +acpi_os_printf("%16.16s % 10d% 10d\n", "TOTALS:",
> acpi_gbl_num_nodes, acpi_gbl_num_objects);
> break;
>
> @@ -415,16 +415,16 @@ acpi_status acpi_db_display_statistics(char *type_arg)
> case CMD_STAT_MISC:
>
> acpi_os_printf("\nMiscellaneous Statistics:\n\n");
> -acpi_os_printf("Calls to AcpiPsFind:.. ........% 7ld\n",
> +acpi_os_printf("Calls to AcpiPsFind:.. ........% 7u\n",
> acpi_gbl_ps_find_count);
> -acpi_os_printf("Calls to AcpiNsLookup:..........% 7ld\n",
> +acpi_os_printf("Calls to AcpiNsLookup:..........% 7u\n",
> acpi_gbl_ns_lookup_count);
>
> acpi_os_printf("\n");
>
> acpi_os_printf("Mutex usage:\n\n");
> for (i = 0; i < ACPI_NUM_MUTEX; i++) {
> -acpi_os_printf("%-28s: % 7ld\n",
> +acpi_os_printf("%-28s: % 7u\n",
> acpi_ut_get_mutex_name(i),
> acpi_gbl_mutex_info[i].use_count);
> }
> @@ -434,87 +434,87 @@ acpi_status acpi_db_display_statistics(char *type_arg)
>
> acpi_os_printf("\nInternal object sizes:\n\n");
>
> -acpi_os_printf("Common %3d\n",
> +acpi_os_printf("Common %3lu\n",
> sizeof(struct acpi_object_common));
> -acpi_os_printf("Number %3d\n",
> +acpi_os_printf("Number %3lu\n",
> sizeof(struct acpi_object_integer));
> -acpi_os_printf("String %3d\n",
> +acpi_os_printf("String %3lu\n",
> sizeof(struct acpi_object_string));
> -acpi_os_printf("Buffer %3d\n",
> +acpi_os_printf("Buffer %3lu\n",
> sizeof(struct acpi_object_buffer));
> -acpi_os_printf("Package %3d\n",
> +acpi_os_printf("Package %3lu\n",
> sizeof(struct acpi_object_package));
> -acpi_os_printf("BufferField %3d\n",
> +acpi_os_printf("BufferField %3lu\n",
> sizeof(struct acpi_object_buffer_field));
> -acpi_os_printf("Device %3d\n",
> +acpi_os_printf("Device %3lu\n",
> sizeof(struct acpi_object_device));
> -acpi_os_printf("Event %3d\n",
> +acpi_os_printf("Event %3lu\n",
> sizeof(struct acpi_object_event));
> -acpi_os_printf("Method %3d\n",
> +acpi_os_printf("Method %3lu\n",
> sizeof(struct acpi_object_method));
> -acpi_os_printf("Mutex %3d\n",
> +acpi_os_printf("Mutex %3lu\n",
> sizeof(struct acpi_object_mutex));
> -acpi_os_printf("Region %3d\n",
> +acpi_os_printf("Region %3lu\n",
> sizeof(struct acpi_object_region));
> -acpi_os_printf("PowerResource %3d\n",
> +acpi_os_printf("PowerResource %3lu\n",
> sizeof(struct acpi_object_power_resource));
> -acpi_os_printf("Processor %3d\n",
> +acpi_os_printf("Processor %3lu\n",
> sizeof(struct acpi_object_processor));
> -acpi_os_printf("ThermalZone %3d\n",
> +acpi_os_printf("ThermalZone %3lu\n",
> sizeof(struct acpi_object_thermal_zone));
> -acpi_os_printf("RegionField %3d\n",
> +acpi_os_printf("RegionField %3lu\n",
> sizeof(struct acpi_object_region_field));
> -acpi_os_printf("BankField %3d\n",
> +acpi_os_printf("BankField %3lu\n",
> sizeof(struct acpi_object_bank_field));
> -acpi_os_printf("IndexField %3d\n",
> +acpi_os_printf("IndexField %3lu\n",
> sizeof(struct acpi_object_index_field));
> -acpi_os_printf("Reference %3d\n",
> +acpi_os_printf("Reference %3lu\n",
> sizeof(struct acpi_object_reference));
> -acpi_os_printf("Notify %3d\n",
> +acpi_os_printf("Notify %3lu\n",
> sizeof(struct acpi_object_notify_handler));
> -acpi_os_printf("AddressSpace %3d\n",
> +acpi_os_printf("AddressSpace %3lu\n",
> sizeof(struct acpi_object_addr_handler));
> -acpi_os_printf("Extra %3d\n",
> +acpi_os_printf("Extra %3lu\n",
> sizeof(struct acpi_object_extra));
> -acpi_os_printf("Data %3d\n",
> +acpi_os_printf("Data %3lu\n",
> sizeof(struct acpi_object_data));
>
> acpi_os_printf("\n");
>
> -acpi_os_printf("ParseObject %3d\n",
> +acpi_os_printf("ParseObject %3lu\n",
> sizeof(struct acpi_parse_obj_common));
> -acpi_os_printf("ParseObjectNamed %3d\n",
> +acpi_os_printf("ParseObjectNamed %3lu\n",
> sizeof(struct acpi_parse_obj_named));
> -acpi_os_printf("ParseObjectAsl %3d\n",
> +acpi_os_printf("ParseObjectAsl %3lu\n",
> sizeof(struct acpi_parse_obj_asl));
> -acpi_os_printf("OperandObject %3d\n",
> +acpi_os_printf("OperandObject %3lu\n",
> sizeof(union acpi_operand_object));
> -acpi_os_printf("NamespaceNode %3d\n",
> +acpi_os_printf("NamespaceNode %3lu\n",
> sizeof(struct acpi_namespace_node));
> -acpi_os_printf("AcpiObject %3d\n",
> +acpi_os_printf("AcpiObject %3lu\n",
> sizeof(union acpi_object));
>
> acpi_os_printf("\n");
>
> -acpi_os_printf("Generic State %3d\n",
> +acpi_os_printf("Generic State %3lu\n",
> sizeof(union acpi_generic_state));
> -acpi_os_printf("Common State %3d\n",
> +acpi_os_printf("Common State %3lu\n",
> sizeof(struct acpi_common_state));
> -acpi_os_printf("Control State %3d\n",
> +acpi_os_printf("Control State %3lu\n",
> sizeof(struct acpi_control_state));
> -acpi_os_printf("Update State %3d\n",
> +acpi_os_printf("Update State %3lu\n",
> sizeof(struct acpi_update_state));
> -acpi_os_printf("Scope State %3d\n",
> +acpi_os_printf("Scope State %3lu\n",
> sizeof(struct acpi_scope_state));
> -acpi_os_printf("Parse Scope %3d\n",
> +acpi_os_printf("Parse Scope %3lu\n",
> sizeof(struct acpi_pscope_state));
> -acpi_os_printf("Package State %3d\n",
> +acpi_os_printf("Package State %3lu\n",
> sizeof(struct acpi_pkg_state));
> -acpi_os_printf("Thread State %3d\n",
> +acpi_os_printf("Thread State %3lu\n",
> sizeof(struct acpi_thread_state));
> -acpi_os_printf("Result Values %3d\n",
> +acpi_os_printf("Result Values %3lu\n",
> sizeof(struct acpi_result_values));
> -acpi_os_printf("Notify Info %3d\n",
> +acpi_os_printf("Notify Info %3lu\n",
> sizeof(struct acpi_notify_info));
> break;
>
> diff --git a/drivers/acpi/acpica/utdebug.c b/drivers/acpi/acpica/utdebug.c
> index 044df9b0356e..b4cdb9c14a87 100644
> --- a/drivers/acpi/acpica/utdebug.c
> +++ b/drivers/acpi/acpica/utdebug.c
> @@ -189,7 +189,7 @@ acpi_debug_print(u32 requested_debug_level,
> * Display the module name, current line number, thread ID (if requested),
> * current procedure nesting level, and the current procedure name
> */
> -acpi_os_printf("%9s-%04ld ", module_name, line_number);
> +acpi_os_printf("%9s-%04u ", module_name, line_number);
>
> #ifdef ACPI_APPLICATION
> /*
Please split above changes into a separate patch.
I'm not sure if the changes can break other compilers.
Please clone https://github.com/acpica/acpica, and submit a pull request there to have more ACPICA reviewers.
> diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
> index f3414c83abb1..48b21490bbeb 100644
> --- a/include/acpi/acpiosxf.h
> +++ b/include/acpi/acpiosxf.h
> @@ -337,11 +337,12 @@ acpi_status acpi_os_signal(u32 function, void *info);
> * Debug print routines
> */
> #ifndef ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_printf
> +__printf(1, 2)
> void ACPI_INTERNAL_VAR_XFACE acpi_os_printf(const char *format, ...);
> #endif
>
> #ifndef ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_vprintf
> -void acpi_os_vprintf(const char *format, va_list args);
> +__printf(1, 0) void acpi_os_vprintf(const char *format, va_list args);
> #endif
>
> #ifndef ACPI_USE_ALTERNATE_PROTOTYPE_acpi_os_redirect_output
You can use ACPI_PRINTF_LIKE macro instead.
This can directly go into Linux upstream.
You can prepare a different pull request for ACPICA upstream or
I can help to back port the change.
Thanks
Lv
> --
> 2.7.4
>
>
> --
> Kees Cook
> Nexus Security
------------------------------
Subject: Digest Footer
_______________________________________________
Devel mailing list
Devel(a)acpica.org
https://lists.acpica.org/mailman/listinfo/devel
------------------------------
End of Devel Digest, Vol 72, Issue 1
************************************
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
4 years, 5 months
- 3
- 2
Re: [Devel] [PATCH v2] acpi: acpica: fix acpi operand cache leak
by Zheng, Lv
Hi,
> From: linux-acpi-owner(a)vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Seunghun
> Han
> Subject: [PATCH v2] acpi: acpica: fix acpi operand cache leak
>
> 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
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?
>
> 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.
>
> Signed-off-by: Seunghun Han <kkamagui(a)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
Thus this looks OK to me.
>
> /*
> * Free the entire namespace -- all nodes and all objects
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo(a)vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
4 years, 5 months
- 1
- 0
[PATCH v2] EFI: fix build for AARCH64
by Leif Lindholm
AARCH64 build was left out of initial version, so add to .dsc and set
ACPI_MACHINE_WIDTH correctly.
Also, acpidump.inf specified explicit per-architecture (but identical)
CFLAGS. Rather than duplicating this further, use the same setting for
all architectures until there is actually a need to diverge.
Also update README to make it obvious AArch64 is supported.
Signed-off-by: Leif Lindholm <leif.lindholm(a)linaro.org>
---
v2 contains a fix for a non-functional issue, which could generate
warnings when building with CLANG.
generate/efi/AcpiPkg.dsc | 2 +-
generate/efi/README | 1 +
generate/efi/acpidump/acpidump.inf | 6 ++----
source/include/platform/acefi.h | 2 +-
4 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/generate/efi/AcpiPkg.dsc b/generate/efi/AcpiPkg.dsc
index 323d39c..9302555 100644
--- a/generate/efi/AcpiPkg.dsc
+++ b/generate/efi/AcpiPkg.dsc
@@ -18,7 +18,7 @@
PLATFORM_VERSION = 1.0
DSC_SPECIFICATION = 0x00010005
OUTPUT_DIRECTORY = Build/Acpi
- SUPPORTED_ARCHITECTURES = IA32|X64
+ SUPPORTED_ARCHITECTURES = AARCH64|IA32|X64
BUILD_TARGETS = DEBUG|RELEASE
SKUID_IDENTIFIER = DEFAULT
diff --git a/generate/efi/README b/generate/efi/README
index 7550e9d..b3ec3ff 100644
--- a/generate/efi/README
+++ b/generate/efi/README
@@ -20,6 +20,7 @@ But the porting has only been tested in a Linux environment.
You can find built EFI binaries (e.x., acpidump.efi) in the following
folders:
+ Build/Acpi/DEBUG_GCC47/AARCH64: aarch64 targets
Build/Acpi/DEBUG_GCC47/IA32: i386 targets
Build/Acpi/DEBUG_GCC47/X64: x86_64 targets
diff --git a/generate/efi/acpidump/acpidump.inf b/generate/efi/acpidump/acpidump.inf
index 8d2c979..e5f21c0 100644
--- a/generate/efi/acpidump/acpidump.inf
+++ b/generate/efi/acpidump/acpidump.inf
@@ -55,7 +55,5 @@
BaseLib
[BuildOptions]
- MSFT:*_*_IA32_CC_FLAGS = /Oi- /WX- /D_EDK2_EFI /DACPI_DUMP_APP
- MSFT:*_*_X64_CC_FLAGS = /Oi- /WX- /D_EDK2_EFI /DACPI_DUMP_APP
- GCC:*_*_IA32_CC_FLAGS = -U__linux__ -U_LINUX -D_EDK2_EFI -DACPI_DUMP_APP -fno-builtin -iwithprefix include
- GCC:*_*_X64_CC_FLAGS = -U__linux__ -U_LINUX -D_EDK2_EFI -DACPI_DUMP_APP -fno-builtin -iwithprefix include
+ MSFT:*_*_*_CC_FLAGS = /Oi- /WX- /D_EDK2_EFI /DACPI_DUMP_APP
+ GCC:*_*_*_CC_FLAGS = -U__linux__ -U_LINUX -D_EDK2_EFI -DACPI_DUMP_APP -fno-builtin -iwithprefix include
diff --git a/source/include/platform/acefi.h b/source/include/platform/acefi.h
index 8328ab6..75ef89a 100644
--- a/source/include/platform/acefi.h
+++ b/source/include/platform/acefi.h
@@ -140,7 +140,7 @@
#define VOID void
-#if defined(__ia64__) || defined(__x86_64__)
+#if defined(__aarch64__) || defined(__ia64__) || defined(__x86_64__)
#define ACPI_MACHINE_WIDTH 64
--
2.10.2
4 years, 5 months
- 3
- 5
Re: [Devel] [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
by Moore, Robert
We are looking at this. It might also help if you could send a full acpidump for the machine. Thanks.
> -----Original Message-----
> From: João Paulo Rechi Vita [mailto:jprvita@gmail.com]
> Sent: Monday, February 6, 2017 6:46 AM
> To: Zheng, Lv <lv.zheng(a)intel.com>
> Cc: Moore, Robert <robert.moore(a)intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki(a)intel.com>; Len Brown <lenb(a)kernel.org>; Lin Ming
> <ming.m.lin(a)intel.com>; linux-acpi(a)vger.kernel.org; devel(a)acpica.org;
> linux-kernel(a)vger.kernel.org; Daniel Drake <drake(a)endlessm.com>;
> linux(a)endlessm.com; Jo?o Paulo Rechi Vita <jprvita(a)endlessm.com>; Box,
> David E <david.e.box(a)intel.com>; Schmauss, Erik
> <erik.schmauss(a)intel.com>
> Subject: Re: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
>
> On 5 February 2017 at 20:44, Zheng, Lv <lv.zheng(a)intel.com> wrote:
> >> From: Moore, Robert
> >> Subject: RE: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
> >>
> >> Here's the sequence of events as I see it:
> >>
> >> Repair_HID is a standalone function that removes one reference on the
> >> incoming object. For simple _HID objects, this in fact deletes the
> object.
> >>
> >> For _CID, all elements of the package are examined. If a repair was
> >> made on a _HID within the _CID function, one reference on the
> >> original object was removed by Repair_HID. However, since the object
> >> is part of a package, it has an extra reference to reflect this fact.
> >> Thus, in the case in question, the elements of the package all have
> >> at least two references. Repair_HID removes one reference, thus the
> extra RemoveReference is needed in Repair_CID to bring the reference
> count down to zero actually delete the object (in the typical case where
> the object had two references).
> >>
>
> This is not what is happening on this machine. Bellow you can see some
> printks I've added in acpi_ns_repair_CID(), right before and after
> acpi_ns_repair_HID() is called:
>
> [ 0.244942] acpi_ns_repair_CID: calling acpi_ns_repair_HID for
> element ffff9a67b3a44f30, refcount=1 [\_SB.PCI0.I2C1.TPL1._CID]
> [ 0.245072] acpi_ns_repair_CID: returned from acpi_ns_repair_HID
> for element ffff9a67b3a44f30, refcount=0 [\_SB.PCI0.I2C1.TPL1._CID]
> [ 0.245202] acpi_ns_repair_CID: element was replaced by element_ptr
> ffff9a67b3a44828, refcount=1 [\_SB.PCI0.I2C1.TPL1._CID]
>
> Here we would call
> acpi_ut_remove_reference(original_element==ffff9a67b3a44f30), which
> already has refcount==0. Maybe there is a refcount increment missing
> from somewhere else?
>
> > Hi,
> >
> > So if a real problem related to package reference counting is
> triggered, the problem should be fixed elsewhere IMO.
>
> Yes, the real problem is i2c_hid not being probed for the touchpad
> device on this platform (sorry, should have added this information to
> the commit message as well). What brought me to this unref was following
> messages on the kernel log:
>
> [ 0.317002] ACPI Warning: Obj ffffa00472a445e8, Reference Count is
> already zero, cannot decrement
> [ 0.317178] (20160422/utdelete-442)
>
> > See this bug for reference:
> > https://bugs.acpica.org/show_bug.cgi?id=1336
> >
>
> Looks like it could be the same problem, indeed. I'm attaching a kernel
> log with acpi.trace_debug_layer=0x10091 acpi.trace_debug_level=0x107FFF
> acpi.trace_method_name=_SB.PCI0.I2C1.TPL1._CID
> acpi.trace_state=opcode, which is what I was using to investigate the
> problem, and the machine's DSDT. Please let me know if there is any
> other information I can provide to help clarify this issue.
>
> Thanks,
>
> --
> João Paulo Rechi Vita
> http://about.me/jprvita
4 years, 5 months
- 1
- 0
Re: [Devel] [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
by Moore, Robert
Here's the sequence of events as I see it:
Repair_HID is a standalone function that removes one reference on the incoming object. For simple _HID objects, this in fact deletes the object.
For _CID, all elements of the package are examined. If a repair was made on a _HID within the _CID function, one reference on the original object was removed by Repair_HID. However, since the object is part of a package, it has an extra reference to reflect this fact. Thus, in the case in question, the elements of the package all have at least two references. Repair_HID removes one reference, thus the extra RemoveReference is needed in Repair_CID to bring the reference count down to zero actually delete the object (in the typical case where the object had two references).
Bob
> -----Original Message-----
> From: João Paulo Rechi Vita [mailto:jprvita@gmail.com]
> Sent: Friday, February 03, 2017 12:57 PM
> To: Moore, Robert; Zheng, Lv; Wysocki, Rafael J; Len Brown; Lin Ming
> Cc: linux-acpi(a)vger.kernel.org; devel(a)acpica.org; linux-
> kernel(a)vger.kernel.org; Daniel Drake; linux(a)endlessm.com; João Paulo Rechi
> Vita
> Subject: [PATCH] acpica: Fix double-free in acpi_ns_repair_CID()
>
> When acpi_ns_repair_CID() is called for a _CID which returns a package of
> strings, it calls acpi_ns_repair_HID() for each of the package elements.
> acpi_ns_repair_HID() calls acpi_ut_remove_reference() on the original
> object, but acpi_ns_repair_CID() calls it again on return, leading to a
> double free.
>
> This problem was seen on a Acer TravelMate P449-G2-MG.
>
> Thanks to Daniel Drake for helping investigating this problem.
>
> Signed-off-by: João Paulo Rechi Vita <jprvita(a)endlessm.com>
> ---
> drivers/acpi/acpica/nsrepair2.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/acpi/acpica/nsrepair2.c
> b/drivers/acpi/acpica/nsrepair2.c index d5336122486b..c429c8eca476 100644
> --- a/drivers/acpi/acpica/nsrepair2.c
> +++ b/drivers/acpi/acpica/nsrepair2.c
> @@ -411,8 +411,6 @@ acpi_ns_repair_CID(struct acpi_evaluate_info *info,
>
> (*element_ptr)->common.reference_count =
> original_ref_count;
> -
> - acpi_ut_remove_reference(original_element);
> }
>
> element_ptr++;
> --
> 2.11.0
4 years, 5 months
- 2
- 1
- ← Newer
- 1
- Older →