* [Qemu-devel] [PATCH] acpi: fix file size check with -acpitable.
@ 2010-07-29 9:08 Isaku Yamahata
2010-08-24 5:06 ` Isaku Yamahata
0 siblings, 1 reply; 3+ messages in thread
From: Isaku Yamahata @ 2010-07-29 9:08 UTC (permalink / raw)
To: qemu-devel
acpi table file can be modified during load so file size check
should be more strict.
pointer calculation should be after qemu_realloc(). not before realloc().
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
hw/acpi.c | 28 +++++++++++++++++++---------
1 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/hw/acpi.c b/hw/acpi.c
index c7044b1..069e05f 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -50,6 +50,8 @@ int acpi_table_add(const char *t)
char buf[1024], *p, *f;
struct acpi_table_header acpi_hdr;
unsigned long val;
+ uint32_t length;
+ struct acpi_table_header *acpi_hdr_p;
size_t off;
memset(&acpi_hdr, 0, sizeof(acpi_hdr));
@@ -108,7 +110,7 @@ int acpi_table_add(const char *t)
buf[0] = '\0';
}
- acpi_hdr.length = sizeof(acpi_hdr);
+ length = sizeof(acpi_hdr);
f = buf;
while (buf[0]) {
@@ -120,7 +122,7 @@ int acpi_table_add(const char *t)
fprintf(stderr, "Can't stat file '%s': %s\n", f, strerror(errno));
goto out;
}
- acpi_hdr.length += s.st_size;
+ length += s.st_size;
if (!n)
break;
*n = ':';
@@ -131,12 +133,12 @@ int acpi_table_add(const char *t)
acpi_tables_len = sizeof(uint16_t);
acpi_tables = qemu_mallocz(acpi_tables_len);
}
+ acpi_tables = qemu_realloc(acpi_tables,
+ acpi_tables_len + sizeof(uint16_t) + length);
p = acpi_tables + acpi_tables_len;
- acpi_tables_len += sizeof(uint16_t) + acpi_hdr.length;
- acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len);
+ acpi_tables_len += sizeof(uint16_t) + length;
- acpi_hdr.length = cpu_to_le32(acpi_hdr.length);
- *(uint16_t*)p = acpi_hdr.length;
+ *(uint16_t*)p = cpu_to_le32(length);
p += sizeof(uint16_t);
memcpy(p, &acpi_hdr, sizeof(acpi_hdr));
off = sizeof(acpi_hdr);
@@ -157,7 +159,9 @@ int acpi_table_add(const char *t)
goto out;
}
- do {
+ /* off < length is necessary because file size can be changed
+ under our foot */
+ while(s.st_size && off < length); {
int r;
r = read(fd, p + off, s.st_size);
if (r > 0) {
@@ -167,15 +171,21 @@ int acpi_table_add(const char *t)
close(fd);
goto out;
}
- } while(s.st_size);
+ }
close(fd);
if (!n)
break;
f = n + 1;
}
+ if (off < length) {
+ /* don't pass random value in process to guest */
+ memset(p + off, 0, length - off);
+ }
- ((struct acpi_table_header*)p)->checksum = acpi_checksum((uint8_t*)p, off);
+ acpi_hdr_p = (struct acpi_table_header*)p;
+ acpi_hdr_p->length = cpu_to_le32(length);
+ acpi_hdr_p->checksum = acpi_checksum((uint8_t*)p, length);
/* increase number of tables */
(*(uint16_t*)acpi_tables) =
cpu_to_le32(le32_to_cpu(*(uint16_t*)acpi_tables) + 1);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: fix file size check with -acpitable.
2010-07-29 9:08 [Qemu-devel] [PATCH] acpi: fix file size check with -acpitable Isaku Yamahata
@ 2010-08-24 5:06 ` Isaku Yamahata
2010-08-31 18:27 ` Blue Swirl
0 siblings, 1 reply; 3+ messages in thread
From: Isaku Yamahata @ 2010-08-24 5:06 UTC (permalink / raw)
To: qemu-devel
ping.
On Thu, Jul 29, 2010 at 06:08:42PM +0900, Isaku Yamahata wrote:
> acpi table file can be modified during load so file size check
> should be more strict.
> pointer calculation should be after qemu_realloc(). not before realloc().
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
> hw/acpi.c | 28 +++++++++++++++++++---------
> 1 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/hw/acpi.c b/hw/acpi.c
> index c7044b1..069e05f 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -50,6 +50,8 @@ int acpi_table_add(const char *t)
> char buf[1024], *p, *f;
> struct acpi_table_header acpi_hdr;
> unsigned long val;
> + uint32_t length;
> + struct acpi_table_header *acpi_hdr_p;
> size_t off;
>
> memset(&acpi_hdr, 0, sizeof(acpi_hdr));
> @@ -108,7 +110,7 @@ int acpi_table_add(const char *t)
> buf[0] = '\0';
> }
>
> - acpi_hdr.length = sizeof(acpi_hdr);
> + length = sizeof(acpi_hdr);
>
> f = buf;
> while (buf[0]) {
> @@ -120,7 +122,7 @@ int acpi_table_add(const char *t)
> fprintf(stderr, "Can't stat file '%s': %s\n", f, strerror(errno));
> goto out;
> }
> - acpi_hdr.length += s.st_size;
> + length += s.st_size;
> if (!n)
> break;
> *n = ':';
> @@ -131,12 +133,12 @@ int acpi_table_add(const char *t)
> acpi_tables_len = sizeof(uint16_t);
> acpi_tables = qemu_mallocz(acpi_tables_len);
> }
> + acpi_tables = qemu_realloc(acpi_tables,
> + acpi_tables_len + sizeof(uint16_t) + length);
> p = acpi_tables + acpi_tables_len;
> - acpi_tables_len += sizeof(uint16_t) + acpi_hdr.length;
> - acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len);
> + acpi_tables_len += sizeof(uint16_t) + length;
>
> - acpi_hdr.length = cpu_to_le32(acpi_hdr.length);
> - *(uint16_t*)p = acpi_hdr.length;
> + *(uint16_t*)p = cpu_to_le32(length);
> p += sizeof(uint16_t);
> memcpy(p, &acpi_hdr, sizeof(acpi_hdr));
> off = sizeof(acpi_hdr);
> @@ -157,7 +159,9 @@ int acpi_table_add(const char *t)
> goto out;
> }
>
> - do {
> + /* off < length is necessary because file size can be changed
> + under our foot */
> + while(s.st_size && off < length); {
> int r;
> r = read(fd, p + off, s.st_size);
> if (r > 0) {
> @@ -167,15 +171,21 @@ int acpi_table_add(const char *t)
> close(fd);
> goto out;
> }
> - } while(s.st_size);
> + }
>
> close(fd);
> if (!n)
> break;
> f = n + 1;
> }
> + if (off < length) {
> + /* don't pass random value in process to guest */
> + memset(p + off, 0, length - off);
> + }
>
> - ((struct acpi_table_header*)p)->checksum = acpi_checksum((uint8_t*)p, off);
> + acpi_hdr_p = (struct acpi_table_header*)p;
> + acpi_hdr_p->length = cpu_to_le32(length);
> + acpi_hdr_p->checksum = acpi_checksum((uint8_t*)p, length);
> /* increase number of tables */
> (*(uint16_t*)acpi_tables) =
> cpu_to_le32(le32_to_cpu(*(uint16_t*)acpi_tables) + 1);
> --
> 1.7.1.1
>
--
yamahata
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] acpi: fix file size check with -acpitable.
2010-08-24 5:06 ` Isaku Yamahata
@ 2010-08-31 18:27 ` Blue Swirl
0 siblings, 0 replies; 3+ messages in thread
From: Blue Swirl @ 2010-08-31 18:27 UTC (permalink / raw)
To: Isaku Yamahata; +Cc: qemu-devel
Thanks, applied.
On Tue, Aug 24, 2010 at 5:06 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> ping.
>
> On Thu, Jul 29, 2010 at 06:08:42PM +0900, Isaku Yamahata wrote:
>> acpi table file can be modified during load so file size check
>> should be more strict.
>> pointer calculation should be after qemu_realloc(). not before realloc().
>>
>> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>> ---
>> hw/acpi.c | 28 +++++++++++++++++++---------
>> 1 files changed, 19 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/acpi.c b/hw/acpi.c
>> index c7044b1..069e05f 100644
>> --- a/hw/acpi.c
>> +++ b/hw/acpi.c
>> @@ -50,6 +50,8 @@ int acpi_table_add(const char *t)
>> char buf[1024], *p, *f;
>> struct acpi_table_header acpi_hdr;
>> unsigned long val;
>> + uint32_t length;
>> + struct acpi_table_header *acpi_hdr_p;
>> size_t off;
>>
>> memset(&acpi_hdr, 0, sizeof(acpi_hdr));
>> @@ -108,7 +110,7 @@ int acpi_table_add(const char *t)
>> buf[0] = '\0';
>> }
>>
>> - acpi_hdr.length = sizeof(acpi_hdr);
>> + length = sizeof(acpi_hdr);
>>
>> f = buf;
>> while (buf[0]) {
>> @@ -120,7 +122,7 @@ int acpi_table_add(const char *t)
>> fprintf(stderr, "Can't stat file '%s': %s\n", f, strerror(errno));
>> goto out;
>> }
>> - acpi_hdr.length += s.st_size;
>> + length += s.st_size;
>> if (!n)
>> break;
>> *n = ':';
>> @@ -131,12 +133,12 @@ int acpi_table_add(const char *t)
>> acpi_tables_len = sizeof(uint16_t);
>> acpi_tables = qemu_mallocz(acpi_tables_len);
>> }
>> + acpi_tables = qemu_realloc(acpi_tables,
>> + acpi_tables_len + sizeof(uint16_t) + length);
>> p = acpi_tables + acpi_tables_len;
>> - acpi_tables_len += sizeof(uint16_t) + acpi_hdr.length;
>> - acpi_tables = qemu_realloc(acpi_tables, acpi_tables_len);
>> + acpi_tables_len += sizeof(uint16_t) + length;
>>
>> - acpi_hdr.length = cpu_to_le32(acpi_hdr.length);
>> - *(uint16_t*)p = acpi_hdr.length;
>> + *(uint16_t*)p = cpu_to_le32(length);
>> p += sizeof(uint16_t);
>> memcpy(p, &acpi_hdr, sizeof(acpi_hdr));
>> off = sizeof(acpi_hdr);
>> @@ -157,7 +159,9 @@ int acpi_table_add(const char *t)
>> goto out;
>> }
>>
>> - do {
>> + /* off < length is necessary because file size can be changed
>> + under our foot */
>> + while(s.st_size && off < length); {
>> int r;
>> r = read(fd, p + off, s.st_size);
>> if (r > 0) {
>> @@ -167,15 +171,21 @@ int acpi_table_add(const char *t)
>> close(fd);
>> goto out;
>> }
>> - } while(s.st_size);
>> + }
>>
>> close(fd);
>> if (!n)
>> break;
>> f = n + 1;
>> }
>> + if (off < length) {
>> + /* don't pass random value in process to guest */
>> + memset(p + off, 0, length - off);
>> + }
>>
>> - ((struct acpi_table_header*)p)->checksum = acpi_checksum((uint8_t*)p, off);
>> + acpi_hdr_p = (struct acpi_table_header*)p;
>> + acpi_hdr_p->length = cpu_to_le32(length);
>> + acpi_hdr_p->checksum = acpi_checksum((uint8_t*)p, length);
>> /* increase number of tables */
>> (*(uint16_t*)acpi_tables) =
>> cpu_to_le32(le32_to_cpu(*(uint16_t*)acpi_tables) + 1);
>> --
>> 1.7.1.1
>>
>
> --
> yamahata
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-08-31 18:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-29 9:08 [Qemu-devel] [PATCH] acpi: fix file size check with -acpitable Isaku Yamahata
2010-08-24 5:06 ` Isaku Yamahata
2010-08-31 18:27 ` Blue Swirl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).