Skip to contentSkip to conversation

buffer array one byte too short in drivers/acpi/system.c

PATCH In review Johann Felix Soden 2 participants 4 messages Mar 6, 2008 – Mar 12, 2008 ACPI

lore
Share & tools
lore

Johann Felix Soden

author
#0 lore raw

PATCH 1/1 buffer array one byte too short in drivers/acpi/system.c

From: Johann Felix Soden <[email protected]>

Since "ff_gbl_lock" has a length of 10 chars and is copied with sprintf to char buffer[10], there is a problem because of the closing zero byte. We need char buffer[11].

Signed-off-by Johann Felix Soden <[email protected]>
---
 drivers/acpi/system.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/acpi/system.c +1 -1
index 55cf4c0..037b679 100644
@@ -319,7 +319,7 @@ void acpi_irq_stats_init(void)
 		goto fail;
 
 	for (i = 0; i < num_counters; ++i) {
-		char buffer[10];
+		char buffer[11];
 		char *name;
 
 		if (i < num_gpes)
Signature · 1 line
1.5.4.3
reviewer
#1 lore raw
Quoted 5 lines · Johann Felix Soden

From: Johann Felix Soden <[email protected]>

Since "ff_gbl_lock" has a length of 10 chars and is copied with sprintf to char buffer[10], there is a problem because of the closing zero byte. We need char buffer[11].

"ff_gbl_lock" is 11 characters, and sprintf adds a null, so buffer should be 12, yes?

looking at this code, i think i must have been asleep when i wrote it. the strings such as "ff_gbl_lock" are in kernel data, and then we copy to a local stack buffer, and then we copy to a heap buffer -- when we should have just pointed to the data.
oh well, cleanup for .26...

Quoted 19 lines
Signed-off-by Johann Felix Soden <[email protected]>
---
 drivers/acpi/system.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/acpi/system.c +1 -1
index 55cf4c0..037b679 100644
@@ -319,7 +319,7 @@ void acpi_irq_stats_init(void)
 		goto fail;
 
 	for (i = 0; i < num_counters; ++i) {
-		char buffer[10];
+		char buffer[11];
 		char *name;
 
 		if (i < num_gpes)
author
#2 lore raw
Quoted 9 lines · Len Brown
Quoted 5 lines · Johann Felix Soden

From: Johann Felix Soden <[email protected]>

Since "ff_gbl_lock" has a length of 10 chars and is copied with sprintf to char buffer[10], there is a problem because of the closing zero byte. We need char buffer[11].

"ff_gbl_lock" is 11 characters, and sprintf adds a null, so buffer should be 12, yes?

I have noticed my miscounting and have sent a new patch to you, Len Brown, some days ago. If it is needed anymore, below is the corrected patch again.

---
From: Johann Felix Soden <[email protected]>

Since "ff_gbl_lock" has a length of 11 chars and is copied with sprintf to char buffer[10], there is a problem. We need char buffer[12] because of the closing zero byte.

Signed-off-by Johann Felix Soden <[email protected]>
---
 drivers/acpi/system.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/acpi/system.c +1 -1
index 55cf4c0..4749f37 100644
@@ -319,7 +319,7 @@ void acpi_irq_stats_init(void)
                goto fail;
 
        for (i = 0; i < num_counters; ++i) {
-               char buffer[10];
+               char buffer[12];
                char *name;
 
                if (i < num_gpes)
Signature · 1 line
1.5.4.3
reviewer
#3 lore raw
Quoted 32 lines · Johann Felix Soden
Quoted 9 lines · Len Brown
Quoted 5 lines · Johann Felix Soden

From: Johann Felix Soden <[email protected]>

Since "ff_gbl_lock" has a length of 10 chars and is copied with sprintf to char buffer[10], there is a problem because of the closing zero byte. We need char buffer[11].

"ff_gbl_lock" is 11 characters, and sprintf adds a null, so buffer should be 12, yes?

I have noticed my miscounting and have sent a new patch to you, Len Brown, some days ago. If it is needed anymore, below is the corrected patch again.

---
From: Johann Felix Soden <[email protected]>

Since "ff_gbl_lock" has a length of 11 chars and is copied with sprintf to char buffer[10], there is a problem. We need char buffer[12] because of the closing zero byte.

Signed-off-by Johann Felix Soden <[email protected]>
---
 drivers/acpi/system.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/acpi/system.c +0 -0
index 55cf4c0..4749f37 100644

drivers/acpi/system.c void acpi_irq_stats_init(void) · line 325

Quoted 8 lines
drivers/acpi/system.c +1 -1
@@ -319,7 +319,7 @@ void acpi_irq_stats_init(void)
                goto fail;
 
        for (i = 0; i < num_counters; ++i) {
-               char buffer[10];
+               char buffer[12];
                char *name;
 
                if (i < num_gpes)

thanks for re-sending, i fixed the whitespace damage and checked it in.

BTW. the reason i didn't see your private re-send to my intel.com account is b/c my intel.com mailbox is not the ideal place to send patches. per http://www.lesswatts.org/projects/acpi/submitting-patches.php
I see patches send to the list, and patches sent to my kernel.org account.

thanks,
-Len