[PATCH] linux-user: Fix parse_elf_properties GNU0_MAGIC check

Richard Henderson posted 1 patch 1 month, 2 weeks ago
linux-user/elfload.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
[PATCH] linux-user: Fix parse_elf_properties GNU0_MAGIC check
Posted by Richard Henderson 1 month, 2 weeks ago
Comparing a string of 4 bytes only works in little-endian.

Adjust bulk bswap to only apply to the note payload.
Perform swapping of the note header manually; the magic
is defined so that it does not need a runtime swap.

Fixes: 83f990eb5adb ("linux-user/elfload: Parse NT_GNU_PROPERTY_TYPE_0 notes")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2596
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 0678c9d506..52c88a68a9 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3121,11 +3121,11 @@ static bool parse_elf_properties(const ImageSource *src,
     }
 
     /*
-     * The contents of a valid PT_GNU_PROPERTY is a sequence
-     * of uint32_t -- swap them all now.
+     * The contents of a valid PT_GNU_PROPERTY is a sequence of uint32_t.
+     * Swap most of them now, beyond the header and namesz.
      */
 #ifdef BSWAP_NEEDED
-    for (int i = 0; i < n / 4; i++) {
+    for (int i = 4; i < n / 4; i++) {
         bswap32s(note.data + i);
     }
 #endif
@@ -3135,15 +3135,15 @@ static bool parse_elf_properties(const ImageSource *src,
      * immediately follows nhdr and is thus at the 4th word.  Further, all
      * of the inputs to the kernel's round_up are multiples of 4.
      */
-    if (note.nhdr.n_type != NT_GNU_PROPERTY_TYPE_0 ||
-        note.nhdr.n_namesz != NOTE_NAME_SZ ||
+    if (tswap32(note.nhdr.n_type) != NT_GNU_PROPERTY_TYPE_0 ||
+        tswap32(note.nhdr.n_namesz) != NOTE_NAME_SZ ||
         note.data[3] != GNU0_MAGIC) {
         error_setg(errp, "Invalid note in PT_GNU_PROPERTY");
         return false;
     }
     off = sizeof(note.nhdr) + NOTE_NAME_SZ;
 
-    datasz = note.nhdr.n_descsz + off;
+    datasz = tswap32(note.nhdr.n_descsz) + off;
     if (datasz > n) {
         error_setg(errp, "Invalid note size in PT_GNU_PROPERTY");
         return false;
-- 
2.43.0
Re: [PATCH] linux-user: Fix parse_elf_properties GNU0_MAGIC check
Posted by Michael Tokarev 1 month, 2 weeks ago
05.10.2024 19:48, Richard Henderson wrote:
> Comparing a string of 4 bytes only works in little-endian.
> 
> Adjust bulk bswap to only apply to the note payload.
> Perform swapping of the note header manually; the magic
> is defined so that it does not need a runtime swap.

I think this is more confusing than my version, but should do the
trick too.

I wasn't sure for my patch because I missed the

  for (int i = 0; i < n / 4; i++) { bswap32s() }

loop, - now I see where it gets performs the conversion, and see why my
patch works (and why it should work too) :)

Either way,

Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>

Also noted for qemu-stable.

Thanks,

/mjt

> Fixes: 83f990eb5adb ("linux-user/elfload: Parse NT_GNU_PROPERTY_TYPE_0 notes")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2596
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/elfload.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 0678c9d506..52c88a68a9 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3121,11 +3121,11 @@ static bool parse_elf_properties(const ImageSource *src,
>       }
>   
>       /*
> -     * The contents of a valid PT_GNU_PROPERTY is a sequence
> -     * of uint32_t -- swap them all now.
> +     * The contents of a valid PT_GNU_PROPERTY is a sequence of uint32_t.
> +     * Swap most of them now, beyond the header and namesz.
>        */
>   #ifdef BSWAP_NEEDED
> -    for (int i = 0; i < n / 4; i++) {
> +    for (int i = 4; i < n / 4; i++) {
>           bswap32s(note.data + i);
>       }
>   #endif
> @@ -3135,15 +3135,15 @@ static bool parse_elf_properties(const ImageSource *src,
>        * immediately follows nhdr and is thus at the 4th word.  Further, all
>        * of the inputs to the kernel's round_up are multiples of 4.
>        */
> -    if (note.nhdr.n_type != NT_GNU_PROPERTY_TYPE_0 ||
> -        note.nhdr.n_namesz != NOTE_NAME_SZ ||
> +    if (tswap32(note.nhdr.n_type) != NT_GNU_PROPERTY_TYPE_0 ||
> +        tswap32(note.nhdr.n_namesz) != NOTE_NAME_SZ ||
>           note.data[3] != GNU0_MAGIC) {
>           error_setg(errp, "Invalid note in PT_GNU_PROPERTY");
>           return false;
>       }
>       off = sizeof(note.nhdr) + NOTE_NAME_SZ;
>   
> -    datasz = note.nhdr.n_descsz + off;
> +    datasz = tswap32(note.nhdr.n_descsz) + off;
>       if (datasz > n) {
>           error_setg(errp, "Invalid note size in PT_GNU_PROPERTY");
>           return false;

-- 
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt