[Qemu-devel] [PATCH for-2.11] linux-user: Fix calculation of auxv length

Peter Maydell posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1510079118-3923-1-git-send-email-peter.maydell@linaro.org
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
linux-user/elfload.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH for-2.11] linux-user: Fix calculation of auxv length
Posted by Peter Maydell 6 years, 4 months ago
In commit 7c4ee5bcc82e643 we changed the order in which we construct
the AUXV, but forgot to adjust the calculation of the length. The
result is that we set info->auxv_len to a bogus and negative value,
and then later on the code in open_self_auxv() gets confused and
ends up presenting the guest with an empty file.

Since we now have to calculate the auxv length up-front as part
of figuring out how much we're going to put on the stack, set
info->auxv_len then; this allows us to assert that we put the
same number of entries into auxv as we pre-calculated, rather
than merely having a comment saying we need to do that.

Fixes: https://bugs.launchpad.net/qemu/+bug/1728116

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/elfload.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 3b857fb..20f3d8c 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1732,6 +1732,8 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
 #ifdef ELF_HWCAP2
     size += 2;
 #endif
+    info->auxv_len = size * n;
+
     size += envc + argc + 2;
     size += 1;  /* argc itself */
     size *= n;
@@ -1760,7 +1762,6 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
         put_user_ual(val, u_auxv); u_auxv += n; \
     } while(0)
 
-    /* There must be exactly DLINFO_ITEMS entries here.  */
 #ifdef ARCH_DLINFO
     /*
      * ARCH_DLINFO must come first so platform specific code can enforce
@@ -1768,6 +1769,9 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
      */
     ARCH_DLINFO;
 #endif
+    /* There must be exactly DLINFO_ITEMS entries here, or the assert
+     * on info->auxv_len will trigger.
+     */
     NEW_AUX_ENT(AT_PHDR, (abi_ulong)(info->load_addr + exec->e_phoff));
     NEW_AUX_ENT(AT_PHENT, (abi_ulong)(sizeof (struct elf_phdr)));
     NEW_AUX_ENT(AT_PHNUM, (abi_ulong)(exec->e_phnum));
@@ -1793,7 +1797,10 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
     NEW_AUX_ENT (AT_NULL, 0);
 #undef NEW_AUX_ENT
 
-    info->auxv_len = u_argv - info->saved_auxv;
+    /* Check that our initial calculation of the auxv length matches how much
+     * we actually put into it.
+     */
+    assert(info->auxv_len == u_auxv - info->saved_auxv);
 
     put_user_ual(argc, u_argc);
 
-- 
2.7.4


Re: [Qemu-devel] [PATCH for-2.11] linux-user: Fix calculation of auxv length
Posted by Richard Henderson 6 years, 4 months ago
On 11/07/2017 07:25 PM, Peter Maydell wrote:
> In commit 7c4ee5bcc82e643 we changed the order in which we construct
> the AUXV, but forgot to adjust the calculation of the length. The
> result is that we set info->auxv_len to a bogus and negative value,
> and then later on the code in open_self_auxv() gets confused and
> ends up presenting the guest with an empty file.
> 
> Since we now have to calculate the auxv length up-front as part
> of figuring out how much we're going to put on the stack, set
> info->auxv_len then; this allows us to assert that we put the
> same number of entries into auxv as we pre-calculated, rather
> than merely having a comment saying we need to do that.
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1728116
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/elfload.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH for-2.11] linux-user: Fix calculation of auxv length
Posted by Peter Maydell 6 years, 4 months ago
On 8 November 2017 at 08:41, Richard Henderson <rth@twiddle.net> wrote:
> On 11/07/2017 07:25 PM, Peter Maydell wrote:
>> In commit 7c4ee5bcc82e643 we changed the order in which we construct
>> the AUXV, but forgot to adjust the calculation of the length. The
>> result is that we set info->auxv_len to a bogus and negative value,
>> and then later on the code in open_self_auxv() gets confused and
>> ends up presenting the guest with an empty file.
>>
>> Since we now have to calculate the auxv length up-front as part
>> of figuring out how much we're going to put on the stack, set
>> info->auxv_len then; this allows us to assert that we put the
>> same number of entries into auxv as we pre-calculated, rather
>> than merely having a comment saying we need to do that.
>>
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1728116
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  linux-user/elfload.c | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Ping! This is a for-2.11 patch with review, and rc2 is tomorrow:
Riku, are you planning a pull request?

thanks
-- PMM