linux-user: Do not subtract offset from end address
PATCH 1/1 linux-user: Do not subtract offset from end address
When computing the upper address of a program segment, do not subtract the offset from the virtual address; instead compute the sum of the virtual address and the memory size.
---
Please include this patch in QEMU 2.2.
Commit a93934fecd4dffc9d4b452b670c9506be5dea30d injected a regression of Linux User Mode that I was able to detect on PowerPC 64 (but not x86). I suspect that large page size on the host has something to do with it. In any case, that commit adjusted the lower address of a program segment by the program header's offset field. However, it also inadvertantly adjusted the upper address by the offset also.
linux-user/elfload.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
linux-user/elfload.c +1 -1
@@ -1824,7 +1824,7 @@ static void load_elf_image(const char *image_name, int image_fd,if (a < loaddr) { loaddr = a; } - a += phdr[i].p_memsz; + a = phdr[i].p_vaddr + phdr[i].p_memsz;if (a > hiaddr) { hiaddr = a; }
Signature · 1 line
1.7.1
Quoted 3 lines · Tom Musta
When computing the upper address of a program segment, do not subtract the offset from the virtual address; instead compute the sum of the virtual address and the memory size.
Thanks, I'll test this and try to get it applied ASAP. Jonas, can you have a look and provide your Acked-by/Tested-by ?
Riku
Quoted 29 lines
Signed-off-by Tom Musta <[email protected]>---
Please include this patch in QEMU 2.2.
Commit
a93934fecd4dffc9d4b452b670c9506be5dea30dinjected a regression of Linux User Mode that I was able to detect on PowerPC 64 (but not x86). I suspect that large page size on the host has something to do with it. In any case, that commit adjusted the lower address of a program segment by the program header's offset field. However, it also inadvertantly adjusted the upper address by the offset also.linux-user/elfload.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)linux-user/elfload.c +1
-1@@ -1824,7 +1824,7 @@ static void load_elf_image(const char *image_name, int image_fd,if (a < loaddr) { loaddr = a; } - a += phdr[i].p_memsz; + a = phdr[i].p_vaddr + phdr[i].p_memsz;if (a > hiaddr) { hiaddr = a; }Signature · 1 line
1.7.1
Quoted 7 lines · Riku Voipio
Quoted 3 lines · Tom Musta
When computing the upper address of a program segment, do not subtract the offset from the virtual address; instead compute the sum of the virtual address and the memory size.
Thanks, I'll test this and try to get it applied ASAP. Jonas, can you have a look and provide your Acked-by/Tested-by ?
Good catch! Sorry for not noticing that. I've verified and the patched version also still works with my ARM binary.
While looking at that, I noticed that the code under "#ifdef CONFIG_USE_FDPIC" in linux-user/elfload.c at line 1858 may need a similar adjustment as performed by my original patch. At least http://lxr.free-electrons.com/source/fs/binfmt_elf.c#L829
makes the offset adjustment both for binaries with and without a
"load_bias". I'm not sure what this is for (some uCLinux-specific format?), nor do I have binaries that exercise this functionality, so I can't/won't provide a patch for this.
Thanks,
Jonas
Quoted 30 lines
Quoted 30 lines
Signed-off-by Tom Musta <[email protected]>---
Please include this patch in QEMU 2.2.
Commit
a93934fecd4dffc9d4b452b670c9506be5dea30dinjected a regression of Linux User Mode that I was able to detect on PowerPC 64 (but not x86). I suspect that large page size on the host has something to do with it. In any case, that commit adjusted the lower address of a program segment by the program header's offset field. However, it also inadvertantly adjusted the upper address by the offset also.
linux-user/elfload.c| 2 +-1 files changed, 1 insertions(+), 1 deletions(-)linux-user/elfload.c +1
-1@@ -1824,7 +1824,7 @@ static void load_elf_image(const char *image_name, int image_fd, if (a < loaddr) { loaddr = a; } - a += phdr[i].p_memsz; + a = phdr[i].p_vaddr + phdr[i].p_memsz;if (a > hiaddr) { hiaddr = a;}Signature · 1 line
1.7.1
Am 06.11.2014 um 20:43 schrieb Tom Musta:
Quoted 3 lines
When computing the upper address of a program segment, do not subtract the offset from the virtual address; instead compute the sum of the virtual address and the memory size.
Note that this reads a bit weird as both old and new code are adding, not subtracting.
Regards,
Andreas
Quoted 29 lines
Signed-off-by Tom Musta <[email protected]>---
Please include this patch in QEMU 2.2.
Commit
a93934fecd4dffc9d4b452b670c9506be5dea30dinjected a regression of Linux User Mode that I was able to detect on PowerPC 64 (but not x86). I suspect that large page size on the host has something to do with it. In any case, that commit adjusted the lower address of a program segment by the program header's offset field. However, it also inadvertantly adjusted the upper address by the offset also.linux-user/elfload.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)linux-user/elfload.c +1
-1@@ -1824,7 +1824,7 @@ static void load_elf_image(const char *image_name, int image_fd,if (a < loaddr) { loaddr = a; } - a += phdr[i].p_memsz; + a = phdr[i].p_vaddr + phdr[i].p_memsz;if (a > hiaddr) { hiaddr = a; }
Signature · 2 lines
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
Quoted 11 lines · Andreas Färber
Am 06.11.2014 um 20:43 schrieb Tom Musta:
Quoted 3 lines
When computing the upper address of a program segment, do not subtract the offset from the virtual address; instead compute the sum of the virtual address and the memory size.
Note that this reads a bit weird as both old and new code are adding, not subtracting.
Regards,
Andreas
I agree that it is not obvious from the patch, which needed one more line of context:
abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset;
if (a < loaddr) {
loaddr = a;
}
a = phdr[i].p_vaddr + phdr[i].p_memsz;
if (a > hiaddr) {
hiaddr = a;
}I think the description accurately captures what is being changed in the code. But if you still disagree, I will reword and respin V2.
Quoted 31 lines
Quoted 29 lines
Signed-off-by Tom Musta <[email protected]>---
Please include this patch in QEMU 2.2.
Commit
a93934fecd4dffc9d4b452b670c9506be5dea30dinjected a regression of Linux User Mode that I was able to detect on PowerPC 64 (but not x86). I suspect that large page size on the host has something to do with it. In any case, that commit adjusted the lower address of a program segment by the program header's offset field. However, it also inadvertantly adjusted the upper address by the offset also.linux-user/elfload.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)linux-user/elfload.c +1
-1@@ -1824,7 +1824,7 @@ static void load_elf_image(const char *image_name, int image_fd,if (a < loaddr) { loaddr = a; } - a += phdr[i].p_memsz; + a = phdr[i].p_vaddr + phdr[i].p_memsz;if (a > hiaddr) { hiaddr = a; }