Message ID | 1510079118-3923-1-git-send-email-peter.maydell@linaro.org |
---|---|
State | New |
Headers | show |
Series | [for-2.11] linux-user: Fix calculation of auxv length | expand |
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~
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
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);
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(-)