1. Patchew
  2. QEMU
  3. [PATCH 0/5] Fix use-after-free and make format overflow more difficult
  4. View patch

[PATCH 1/5] contrib/elf2dmp: Grow PDB URL buffer

Akihiko Odaki posted 5 patches 2 weeks, 2 days ago
Maintainers: Viktor Prutyanov <viktor.prutyanov@phystech.edu>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Jesper Devantier <foss@defmacro.it>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>
[PATCH 1/5] contrib/elf2dmp: Grow PDB URL buffer
Posted by Akihiko Odaki 2 weeks, 2 days ago 
The buffers used to construct a PDB URL overflow when the "age" property
is greater than 0xf, so grow it. This also simplifies the logic of the
URL construction to use one buffer instead of two to avoid the chore to
synchronize the sizes of two buffers.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 contrib/elf2dmp/main.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index d046a72ae67f..83ddc57dd9ee 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -494,18 +494,6 @@ static bool pe_check_pdb_name(uint64_t base, void *start_addr,
     return !strcmp(pdb_name, PDB_NAME);
 }
 
-static void pe_get_pdb_symstore_hash(OMFSignatureRSDS *rsds, char *hash)
-{
-    sprintf(hash, "%.08x%.04x%.04x%.02x%.02x", rsds->guid.a, rsds->guid.b,
-            rsds->guid.c, rsds->guid.d[0], rsds->guid.d[1]);
-    hash += 20;
-    for (unsigned int i = 0; i < 6; i++, hash += 2) {
-        sprintf(hash, "%.02x", rsds->guid.e[i]);
-    }
-
-    sprintf(hash, "%.01x", rsds->age);
-}
-
 int main(int argc, char *argv[])
 {
     int err = 1;
@@ -517,9 +505,7 @@ int main(int argc, char *argv[])
     uint64_t KernBase;
     void *nt_start_addr = NULL;
     WinDumpHeader64 header;
-    char pdb_hash[34];
-    char pdb_url[] = SYM_URL_BASE PDB_NAME
-        "/0123456789ABCDEF0123456789ABCDEFx/" PDB_NAME;
+    char pdb_url[sizeof(SYM_URL_BASE PDB_NAME PDB_NAME) + 42];
     struct pdb_reader pdb;
     uint64_t KdDebuggerDataBlock;
     KDDEBUGGER_DATA64 *kdbg;
@@ -583,9 +569,16 @@ int main(int argc, char *argv[])
     printf("KernBase = 0x%016"PRIx64", signature is \'%.2s\'\n", KernBase,
             (char *)nt_start_addr);
 
-    pe_get_pdb_symstore_hash(&rsds, pdb_hash);
+    sprintf(pdb_url,
+            "%s%.08x%.04x%.04x"
+            "%.02x%.02x"
+            "%.02x%.02x%.02x%.02x%.02x%.02x"
+            "%.01x%s",
+            SYM_URL_BASE PDB_NAME "/", rsds.guid.a, rsds.guid.b, rsds.guid.c,
+            rsds.guid.d[0], rsds.guid.d[1],
+            rsds.guid.e[0], rsds.guid.e[1], rsds.guid.e[2], rsds.guid.e[3], rsds.guid.e[4], rsds.guid.e[5],
+            rsds.age, "/" PDB_NAME);
 
-    sprintf(pdb_url, "%s%s/%s/%s", SYM_URL_BASE, PDB_NAME, pdb_hash, PDB_NAME);
     printf("PDB URL is %s\n", pdb_url);
 
     if (!download_url(PDB_NAME, pdb_url)) {

-- 
2.52.0
Re: [PATCH 1/5] contrib/elf2dmp: Grow PDB URL buffer
Posted by Philippe Mathieu-Daudé 2 weeks, 2 days ago 
On 25/1/26 07:42, Akihiko Odaki wrote:
> The buffers used to construct a PDB URL overflow when the "age" property
> is greater than 0xf, so grow it. This also simplifies the logic of the
> URL construction to use one buffer instead of two to avoid the chore to
> synchronize the sizes of two buffers.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   contrib/elf2dmp/main.c | 27 ++++++++++-----------------
>   1 file changed, 10 insertions(+), 17 deletions(-)
> 
> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
> index d046a72ae67f..83ddc57dd9ee 100644
> --- a/contrib/elf2dmp/main.c
> +++ b/contrib/elf2dmp/main.c
> @@ -494,18 +494,6 @@ static bool pe_check_pdb_name(uint64_t base, void *start_addr,
>       return !strcmp(pdb_name, PDB_NAME);
>   }
>   
> -static void pe_get_pdb_symstore_hash(OMFSignatureRSDS *rsds, char *hash)
> -{
> -    sprintf(hash, "%.08x%.04x%.04x%.02x%.02x", rsds->guid.a, rsds->guid.b,
> -            rsds->guid.c, rsds->guid.d[0], rsds->guid.d[1]);
> -    hash += 20;
> -    for (unsigned int i = 0; i < 6; i++, hash += 2) {
> -        sprintf(hash, "%.02x", rsds->guid.e[i]);
> -    }
> -
> -    sprintf(hash, "%.01x", rsds->age);
> -}
> -
>   int main(int argc, char *argv[])
>   {
>       int err = 1;
> @@ -517,9 +505,7 @@ int main(int argc, char *argv[])
>       uint64_t KernBase;
>       void *nt_start_addr = NULL;
>       WinDumpHeader64 header;
> -    char pdb_hash[34];
> -    char pdb_url[] = SYM_URL_BASE PDB_NAME
> -        "/0123456789ABCDEF0123456789ABCDEFx/" PDB_NAME;
> +    char pdb_url[sizeof(SYM_URL_BASE PDB_NAME PDB_NAME) + 42];
>       struct pdb_reader pdb;
>       uint64_t KdDebuggerDataBlock;
>       KDDEBUGGER_DATA64 *kdbg;
> @@ -583,9 +569,16 @@ int main(int argc, char *argv[])
>       printf("KernBase = 0x%016"PRIx64", signature is \'%.2s\'\n", KernBase,
>               (char *)nt_start_addr);
>   
> -    pe_get_pdb_symstore_hash(&rsds, pdb_hash);

I'd rather use g_autofree + g_strdup_printf and drop the _NAME definitions.

> +    sprintf(pdb_url,
> +            "%s%.08x%.04x%.04x"
> +            "%.02x%.02x"
> +            "%.02x%.02x%.02x%.02x%.02x%.02x"
> +            "%.01x%s",
> +            SYM_URL_BASE PDB_NAME "/", rsds.guid.a, rsds.guid.b, rsds.guid.c,
> +            rsds.guid.d[0], rsds.guid.d[1],
> +            rsds.guid.e[0], rsds.guid.e[1], rsds.guid.e[2], rsds.guid.e[3], rsds.guid.e[4], rsds.guid.e[5],
> +            rsds.age, "/" PDB_NAME);
>   
> -    sprintf(pdb_url, "%s%s/%s/%s", SYM_URL_BASE, PDB_NAME, pdb_hash, PDB_NAME);
>       printf("PDB URL is %s\n", pdb_url);
>   
>       if (!download_url(PDB_NAME, pdb_url)) {
>
Re: [PATCH 1/5] contrib/elf2dmp: Grow PDB URL buffer
Posted by Akihiko Odaki 2 weeks, 1 day ago 
On 2026/01/26 1:58, Philippe Mathieu-Daudé wrote:
> On 25/1/26 07:42, Akihiko Odaki wrote:
>> The buffers used to construct a PDB URL overflow when the "age" property
>> is greater than 0xf, so grow it. This also simplifies the logic of the
>> URL construction to use one buffer instead of two to avoid the chore to
>> synchronize the sizes of two buffers.
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>>   contrib/elf2dmp/main.c | 27 ++++++++++-----------------
>>   1 file changed, 10 insertions(+), 17 deletions(-)
>>
>> diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
>> index d046a72ae67f..83ddc57dd9ee 100644
>> --- a/contrib/elf2dmp/main.c
>> +++ b/contrib/elf2dmp/main.c
>> @@ -494,18 +494,6 @@ static bool pe_check_pdb_name(uint64_t base, void 
>> *start_addr,
>>       return !strcmp(pdb_name, PDB_NAME);
>>   }
>> -static void pe_get_pdb_symstore_hash(OMFSignatureRSDS *rsds, char *hash)
>> -{
>> -    sprintf(hash, "%.08x%.04x%.04x%.02x%.02x", rsds->guid.a, rsds- 
>> >guid.b,
>> -            rsds->guid.c, rsds->guid.d[0], rsds->guid.d[1]);
>> -    hash += 20;
>> -    for (unsigned int i = 0; i < 6; i++, hash += 2) {
>> -        sprintf(hash, "%.02x", rsds->guid.e[i]);
>> -    }
>> -
>> -    sprintf(hash, "%.01x", rsds->age);
>> -}
>> -
>>   int main(int argc, char *argv[])
>>   {
>>       int err = 1;
>> @@ -517,9 +505,7 @@ int main(int argc, char *argv[])
>>       uint64_t KernBase;
>>       void *nt_start_addr = NULL;
>>       WinDumpHeader64 header;
>> -    char pdb_hash[34];
>> -    char pdb_url[] = SYM_URL_BASE PDB_NAME
>> -        "/0123456789ABCDEF0123456789ABCDEFx/" PDB_NAME;
>> +    char pdb_url[sizeof(SYM_URL_BASE PDB_NAME PDB_NAME) + 42];
>>       struct pdb_reader pdb;
>>       uint64_t KdDebuggerDataBlock;
>>       KDDEBUGGER_DATA64 *kdbg;
>> @@ -583,9 +569,16 @@ int main(int argc, char *argv[])
>>       printf("KernBase = 0x%016"PRIx64", signature is \'%.2s\'\n", 
>> KernBase,
>>               (char *)nt_start_addr);
>> -    pe_get_pdb_symstore_hash(&rsds, pdb_hash);
> 
> I'd rather use g_autofree + g_strdup_printf and drop the _NAME definitions.

g_autofree + g_strdup_printf will work. PDB_NAME is still useful because 
somehow it is repeated twice in the URL.

> 
>> +    sprintf(pdb_url,
>> +            "%s%.08x%.04x%.04x"
>> +            "%.02x%.02x"
>> +            "%.02x%.02x%.02x%.02x%.02x%.02x"
>> +            "%.01x%s",
>> +            SYM_URL_BASE PDB_NAME "/", rsds.guid.a, rsds.guid.b, 
>> rsds.guid.c,
>> +            rsds.guid.d[0], rsds.guid.d[1],
>> +            rsds.guid.e[0], rsds.guid.e[1], rsds.guid.e[2], 
>> rsds.guid.e[3], rsds.guid.e[4], rsds.guid.e[5],
>> +            rsds.age, "/" PDB_NAME);
>> -    sprintf(pdb_url, "%s%s/%s/%s", SYM_URL_BASE, PDB_NAME, pdb_hash, 
>> PDB_NAME);
>>       printf("PDB URL is %s\n", pdb_url);
>>       if (!download_url(PDB_NAME, pdb_url)) {
>>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.