Bug 11539
| Summary: | empty filename in /sys/firmware/acpi/tables | ||
|---|---|---|---|
| Product: | ACPI | Reporter: | Peter Gruber (nokos) |
| Component: | Other | Assignee: | Zhang Rui (rui.zhang) |
| Status: | CLOSED PATCH_ALREADY_AVAILABLE | ||
| Severity: | low | CC: | acpi-bugzilla |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.26 | Subsystem: | |
| Regression: | --- | Bisected commit-id: | |
| Attachments: |
patch: acpi tables changes, including the fix for this issue
refreshed patch patch: acpi tables changes, refreshed version patch: acpi tables changes, refreshed version, fixed a suspend/resume issue |
||
Reply-To: akpm@linux-foundation.org On Thu, 11 Sep 2008 02:59:49 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote: > http://bugzilla.kernel.org/show_bug.cgi?id=11539 > > Summary: empty filename in /sys/firmware/acpi/tables > Product: ACPI > Version: 2.5 > KernelVersion: 2.6.26 > Platform: All > OS/Version: Linux > Tree: Mainline > Status: NEW > Severity: low > Priority: P1 > Component: Other > AssignedTo: acpi_other@kernel-bugs.osdl.org > ReportedBy: nokos@gmx.net > > > Hardware Environment: > A system with an acpi table with an empty signature > > Problem Description: > Since commit bc45b1d39a925b56796bebf8a397a0491489d85c acpi tables are allowed > to have an empty signature and /sys/firmware/acpi/tables uses the signature > as > filename. Applications using naive recursion through /sys loop forever. A > possible solution would be: > (replacing the zero length filename with the string "NULL") > > > diff a/drivers/acpi/system.c b/drivers/acpi/system.c > --- a/drivers/acpi/system.c > +++ b/drivers/acpi/system.c (argh) Please don't submit patches via bugzilla, especially not copy-n-paste ones. It gets wordwrapped and tabs get replaced with spaces and similar crap. Patches should be submitted via email. I cleaned this patch up and applied it locally. Please send a Signed-off-by: for this patch as per Documentation/SubmittingPatches, section 12. This patch fixes what appears to be a regression in 2.6.26 and looks like it is applicable to both 2.6.26.x and to 2.6.27. Len, Bob: please review asap - I can merge it with your ack if you'd like. From: Peter Gruber <nokos@gmx.net> Taken from http://bugzilla.kernel.org/show_bug.cgi?id=11539 Since commit bc45b1d39a925b56796bebf8a397a0491489d85c acpi tables are allowed to have an empty signature and /sys/firmware/acpi/tables uses the signature as filename. Applications using naive recursion through /sys loop forever. A possible solution would be: (replacing the zero length filename with the string "NULL") Cc: Bob Moore <robert.moore@intel.com> Cc: Lin Ming <ming.m.lin@intel.com> Cc: Len Brown <len.brown@intel.com> Cc: <stable@kernel.org> [2.6.26.x] Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- drivers/acpi/system.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff -puN drivers/acpi/system.c~acpi-dont-create-empty-filenames-in-sys-firmware-acpi-tables drivers/acpi/system.c --- a/drivers/acpi/system.c~acpi-dont-create-empty-filenames-in-sys-firmware-acpi-tables +++ a/drivers/acpi/system.c @@ -78,9 +78,15 @@ static ssize_t acpi_table_show(struct ko container_of(bin_attr, struct acpi_table_attr, attr); struct acpi_table_header *table_header = NULL; acpi_status status; + char name[ACPI_NAME_SIZE]; + + if (strncmp(table_attr->name, "NULL", 4)) + memcpy(name, table_attr->name, ACPI_NAME_SIZE); + else + memcpy(name, "\0\0\0\0", 4); status = - acpi_get_table(table_attr->name, table_attr->instance, + acpi_get_table(name, table_attr->instance, &table_header); if (ACPI_FAILURE(status)) return -ENODEV; @@ -95,21 +101,24 @@ static void acpi_table_attr_init(struct struct acpi_table_header *header = NULL; struct acpi_table_attr *attr = NULL; - memcpy(table_attr->name, table_header->signature, ACPI_NAME_SIZE); + if (table_header->signature[0] != '\0') + memcpy(table_attr->name, table_header->signature, + ACPI_NAME_SIZE); + else + memcpy(table_attr->name, "NULL", 4); list_for_each_entry(attr, &acpi_table_attr_list, node) { - if (!memcmp(table_header->signature, attr->name, - ACPI_NAME_SIZE)) + if (!memcmp(table_attr->name, attr->name, ACPI_NAME_SIZE)) if (table_attr->instance < attr->instance) table_attr->instance = attr->instance; } table_attr->instance++; if (table_attr->instance > 1 || (table_attr->instance == 1 && - !acpi_get_table(table_header-> - signature, 2, - &header))) - sprintf(table_attr->name + 4, "%d", table_attr->instance); + !acpi_get_table + (table_header->signature, 2, &header))) + sprintf(table_attr->name + ACPI_NAME_SIZE, "%d", + table_attr->instance); table_attr->attr.size = 0; table_attr->attr.read = acpi_table_show; _ On Thu, 2008-09-11 at 12:33 -0700, Andrew Morton wrote: > On Thu, 11 Sep 2008 02:59:49 -0700 (PDT) > bugme-daemon@bugzilla.kernel.org wrote: > > > http://bugzilla.kernel.org/show_bug.cgi?id=11539 > > > > Summary: empty filename in /sys/firmware/acpi/tables > > Product: ACPI > > Version: 2.5 > > KernelVersion: 2.6.26 > > Platform: All > > OS/Version: Linux > > Tree: Mainline > > Status: NEW > > Severity: low > > Priority: P1 > > Component: Other > > AssignedTo: acpi_other@kernel-bugs.osdl.org > > ReportedBy: nokos@gmx.net > > > > > > Hardware Environment: > > A system with an acpi table with an empty signature > > > > Problem Description: > > Since commit bc45b1d39a925b56796bebf8a397a0491489d85c acpi tables are > allowed > > to have an empty signature and /sys/firmware/acpi/tables uses the signature > as > > filename. Applications using naive recursion through /sys loop forever. A > > possible solution would be: > > (replacing the zero length filename with the string "NULL") > > Acked-by: Zhang Rui <rui.zhang@intel.com> thanks, rui > > diff a/drivers/acpi/system.c b/drivers/acpi/system.c > > --- a/drivers/acpi/system.c > > +++ b/drivers/acpi/system.c > > (argh) > > > Please don't submit patches via bugzilla, especially not copy-n-paste > ones. It gets wordwrapped and tabs get replaced with spaces and > similar crap. Patches should be submitted via email. > > I cleaned this patch up and applied it locally. > > Please send a Signed-off-by: for this patch as per > Documentation/SubmittingPatches, section 12. > > This patch fixes what appears to be a regression in 2.6.26 and looks > like it is applicable to both 2.6.26.x and to 2.6.27. Len, Bob: please > review asap - I can merge it with your ack if you'd like. > > > > From: Peter Gruber <nokos@gmx.net> > > Taken from http://bugzilla.kernel.org/show_bug.cgi?id=11539 > > Since commit bc45b1d39a925b56796bebf8a397a0491489d85c acpi tables are > allowed to have an empty signature and /sys/firmware/acpi/tables uses the > signature as filename. Applications using naive recursion through /sys > loop forever. A possible solution would be: (replacing the zero length > filename with the string "NULL") > > Cc: Bob Moore <robert.moore@intel.com> > Cc: Lin Ming <ming.m.lin@intel.com> > Cc: Len Brown <len.brown@intel.com> > Cc: <stable@kernel.org> [2.6.26.x] > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > --- > > drivers/acpi/system.c | 25 +++++++++++++++++-------- > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff -puN > drivers/acpi/system.c~acpi-dont-create-empty-filenames-in-sys-firmware-acpi-tables > drivers/acpi/system.c > --- > a/drivers/acpi/system.c~acpi-dont-create-empty-filenames-in-sys-firmware-acpi-tables > +++ a/drivers/acpi/system.c > @@ -78,9 +78,15 @@ static ssize_t acpi_table_show(struct ko > container_of(bin_attr, struct acpi_table_attr, attr); > struct acpi_table_header *table_header = NULL; > acpi_status status; > + char name[ACPI_NAME_SIZE]; > + > + if (strncmp(table_attr->name, "NULL", 4)) > + memcpy(name, table_attr->name, ACPI_NAME_SIZE); > + else > + memcpy(name, "\0\0\0\0", 4); > > status = > - acpi_get_table(table_attr->name, table_attr->instance, > + acpi_get_table(name, table_attr->instance, > &table_header); > if (ACPI_FAILURE(status)) > return -ENODEV; > @@ -95,21 +101,24 @@ static void acpi_table_attr_init(struct > struct acpi_table_header *header = NULL; > struct acpi_table_attr *attr = NULL; > > - memcpy(table_attr->name, table_header->signature, ACPI_NAME_SIZE); > + if (table_header->signature[0] != '\0') > + memcpy(table_attr->name, table_header->signature, > + ACPI_NAME_SIZE); > + else > + memcpy(table_attr->name, "NULL", 4); > > list_for_each_entry(attr, &acpi_table_attr_list, node) { > - if (!memcmp(table_header->signature, attr->name, > - ACPI_NAME_SIZE)) > + if (!memcmp(table_attr->name, attr->name, ACPI_NAME_SIZE)) > if (table_attr->instance < attr->instance) > table_attr->instance = attr->instance; > } > table_attr->instance++; > > if (table_attr->instance > 1 || (table_attr->instance == 1 && > - !acpi_get_table(table_header-> > - signature, 2, > - &header))) > - sprintf(table_attr->name + 4, "%d", table_attr->instance); > + !acpi_get_table > + (table_header->signature, 2, &header))) > + sprintf(table_attr->name + ACPI_NAME_SIZE, "%d", > + table_attr->instance); > > table_attr->attr.size = 0; > table_attr->attr.read = acpi_table_show; > _ > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html What happens when we encouter a 2nd table w/ a null signature, won't we then attempt to create two sysfs files with the same name? well, we will try to create a duplicate sysfs files with the same name in this case. this doesn't sounds good. I'll refresh the patch here and send it out soon. Created attachment 18185 [details] patch: acpi tables changes, including the fix for this issue hi, peter, I made several changes of ACPI table management, including the one for this bug. could you please give it a try? note it's on top of len's acpi test branch. :) how to get the acpi test branch can be found at: http://www.lesswatts.org/projects/acpi/git.php Created attachment 18186 [details]
refreshed patch
(In reply to comment #3) > What happens when we encouter a 2nd table w/ a null signature, > won't we then attempt to create two sysfs files with the same name? this is taken care of in the standard way (adding an index, NULL1, NULL2, ..) like in the case when there are more tahn one SSDT table (SSDT1, SSDT2, ...) I can only think of one (somewhat) unhandled case: when there are more than one table signature starting with \0 but continuing differently (say '\0\0\0\0' and '\0FOO'). In this case multiple files with the same name are probably created. (In reply to comment #5) > hi, peter, > > I made several changes of ACPI table management, including the one for this > bug. > could you please give it a try? with your patch I get the following kernel panic at boot: BUG: unable to handle kernel paging request at ffffc20000039018 IP: [<ffffffff808db96d>] acpi_initialize_inactive_tables+0x1e2/0x280 PGD 12f804067 PUD 12f805067 PMD 12f806067 PTE 0 Oops: 0000 [1] SMP CPU 0 Modules linked in: Pid: 1, comm: swapper Tainted: G W 2.6.27-rc8 #2 RIP: 0010:[<ffffffff808db96d>] [<ffffffff808db96d>] acpi_initialize_inactive_tables+0x1e2/0x280 RSP: 0018:ffff88012f85be10 EFLAGS: 00010282 RAX: ffffc20000038f8c RBX: 0000000000000000 RCX: ffff88012f85bce0 RDX: ffffc20000038000 RSI: 0000000000000000 RDI: ffffc20000038f8c RBP: ffff88012f85be50 R08: 0000000000000000 R09: 00000000000000d0 R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000 R13: ffffc2000003875c R14: ffffc2000003870c R15: 0000000000000050 FS: 0000000000000000(0000) GS: fffffff808a3340(0000) knlGS:0000000000000000 CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b CR2: ffffc20000038018 CR3: 0000000000201000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process swapper (pid: 1, threadinfo ffff88012f85a000, task ffff88012f860000) Stack: 00000000000092fe0 0000000000000000 0000000000000000 ffffffff809073e8 0000000000000000 0000000000000000 0000000000092fe0 0000000000000000 ffff88012f85bea0 ffffffff808dcf32 ffffffff808dcc45 0000000000000000 Call Trace: [<ffffffff808dcf32>] acpi_system_init+0xa7/0x208 [<ffffffff808dcc45>] ? acpi_power_init+0x0/0x77 [<ffffffff808dce8b>] ? acpi_system_init+0x0/0x208 [<ffffffff8020903c>] _stext+0x3c/0x170 [<ffffffff80284103>] ? register_irq_proc+0xd3/0xf0 [<ffffffff80300000>] ? vfs_set_dqblk+0x230/0x360 [<ffffffff808b96b3>] kernel_init+0x141/0x197 [<ffffffff802413f7>] ? schedule_tail+0x27/0x70 [<ffffffff8020d9e9>] child_rip+0xa/0x11 [<ffffffff808b9572>] ? kernel_init+0x0/0x197 [<ffffffff8020d9df>] ? child_rip+0x0/0x11 Code: 0c 00 48 01 da 8b 42 14 3b 05 8a 74 ee ff 0f 85 01 00 00 00 48 8b 3a 8b 72 10 e8 fb cc da ff 48 89 c7 48 85 c0 0f 84 85 00 00 00 <48> 8b 97 8c 00 00 00 48 8b 0d c5 62 0c 00 8b 05 c7 62 0c 00 48 RIP [<fffffffff808db96d>] acpi_initialize_inactive_tables+0x1e2/0x280 RSP <ffff88012f85be10> CR2: ffffc20000039010 ---[ end trace 4eaa2a86a8e2da22 ]--- Created attachment 18235 [details]
patch: acpi tables changes, refreshed version
sorry, would you please try this one please?
Created attachment 18237 [details]
patch: acpi tables changes, refreshed version, fixed a suspend/resume issue
(In reply to comment #10) > Created an attachment (id=18237) [details] > patch: acpi tables changes, refreshed version, fixed a suspend/resume issue ok ... this one works, what do the _inactive, _dynamic mean? _inactive tables are the tables that exported by the inactive root table. i.e. if Linux is using XSDT on this laptop, tables from RSDT are inactive tables. _dynamic tables are the ACPI tables that loaded at runtime, which is different from the tables that are mapped at the boot time(tables from RSDT/XSDT). applied akpm's update to peter's patch to acpi tree. I plan to send it to .26.stable and .27.stable as soon as it hits upstream. shipped in 2.6.28-rc4-git3 commit 4feba70a2c1a1a0c96909f657f48b2e11e682370 Author: Peter Gruber <nokos@gmx.net> Date: Mon Oct 27 23:59:36 2008 -0400 ACPI: avoid empty file name in sysfs e-mailed to 2.6.26-stable, 2.6.27-stable. closed. |
Hardware Environment: A system with an acpi table with an empty signature Problem Description: Since commit bc45b1d39a925b56796bebf8a397a0491489d85c acpi tables are allowed to have an empty signature and /sys/firmware/acpi/tables uses the signature as filename. Applications using naive recursion through /sys loop forever. A possible solution would be: (replacing the zero length filename with the string "NULL") diff a/drivers/acpi/system.c b/drivers/acpi/system.c --- a/drivers/acpi/system.c +++ b/drivers/acpi/system.c @@ -78,9 +78,15 @@ static ssize_t acpi_table_show(struct kobject *kobj, container_of(bin_attr, struct acpi_table_attr, attr); struct acpi_table_header *table_header = NULL; acpi_status status; + char name[ACPI_NAME_SIZE]; + + if (strncmp(table_attr->name, "NULL", 4)) + memcpy(name, table_attr->name, ACPI_NAME_SIZE); + else + memcpy(name, "\0\0\0\0", 4); status = - acpi_get_table(table_attr->name, table_attr->instance, + acpi_get_table(name, table_attr->instance, &table_header); if (ACPI_FAILURE(status)) return -ENODEV; @@ -95,21 +101,24 @@ static void acpi_table_attr_init(struct acpi_table_attr *table_attr, struct acpi_table_header *header = NULL; struct acpi_table_attr *attr = NULL; - memcpy(table_attr->name, table_header->signature, ACPI_NAME_SIZE); + if (table_header->signature[0] != '\0') + memcpy(table_attr->name, table_header->signature, + ACPI_NAME_SIZE); + else + memcpy(table_attr->name, "NULL", 4); list_for_each_entry(attr, &acpi_table_attr_list, node) { - if (!memcmp(table_header->signature, attr->name, - ACPI_NAME_SIZE)) + if (!memcmp(table_attr->name, attr->name, ACPI_NAME_SIZE)) if (table_attr->instance < attr->instance) table_attr->instance = attr->instance; } table_attr->instance++; if (table_attr->instance > 1 || (table_attr->instance == 1 && - !acpi_get_table(table_header-> - signature, 2, - &header))) - sprintf(table_attr->name + 4, "%d", table_attr->instance); + !acpi_get_table + (table_header->signature, 2, &header))) + sprintf(table_attr->name + ACPI_NAME_SIZE, "%d", + table_attr->instance); table_attr->attr.size = 0; table_attr->attr.read = acpi_table_show;