| Message ID | 20200724212314.545877-1-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | linux-user: Ensure mmap_min_addr is non-zero | expand |
Hi! On 7/24/20 11:23 PM, Richard Henderson wrote: > When the chroot does not have /proc mounted, we can read neither > /proc/sys/vm/mmap_min_addr nor /proc/sys/maps. > > The enforcement of mmap_min_addr in the host kernel is done by > the security module, and so does not apply to processes owned > by root. Which leads pgd_find_hole_fallback to succeed in probing > a reservation at address 0. Which confuses pgb_reserved_va to > believe that guest_base has not actually been initialized. > > We don't actually want NULL addresses to become accessible, so > make sure that mmap_min_addr is initialized with a non-zero value. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1888728 > Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/main.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/linux-user/main.c b/linux-user/main.c > index 3597e99bb1..75c9785157 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -758,14 +758,26 @@ int main(int argc, char **argv, char **envp) > > if ((fp = fopen("/proc/sys/vm/mmap_min_addr", "r")) != NULL) { > unsigned long tmp; > - if (fscanf(fp, "%lu", &tmp) == 1) { > + if (fscanf(fp, "%lu", &tmp) == 1 && tmp != 0) { > mmap_min_addr = tmp; > - qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n", mmap_min_addr); > + qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n", > + mmap_min_addr); > } > fclose(fp); > } > } > > + /* > + * We prefer to not make NULL pointers accessible to QEMU. > + * If we're in a chroot with no /proc, fall back to 1 page. > + */ > + if (mmap_min_addr == 0) { > + mmap_min_addr = qemu_host_page_size; > + qemu_log_mask(CPU_LOG_PAGE, > + "host mmap_min_addr=0x%lx (fallback)\n", > + mmap_min_addr); > + } > + > /* > * Prepare copy of argv vector for target. > */ > This fixes the problem for me, therefore: Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaubitz@debian.org `. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Le 24/07/2020 à 23:23, Richard Henderson a écrit : > When the chroot does not have /proc mounted, we can read neither > /proc/sys/vm/mmap_min_addr nor /proc/sys/maps. > > The enforcement of mmap_min_addr in the host kernel is done by > the security module, and so does not apply to processes owned > by root. Which leads pgd_find_hole_fallback to succeed in probing > a reservation at address 0. Which confuses pgb_reserved_va to > believe that guest_base has not actually been initialized. > > We don't actually want NULL addresses to become accessible, so > make sure that mmap_min_addr is initialized with a non-zero value. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1888728 > Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/main.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > Richard, do you want to add this fix in one of your branches PR? In this case: Acked-by: Laurent Vivier <laurent@vivier.eu> Otherwise I can prepare a small one for linux-user, adding one or two fixes... Thanks, Laurent
On 7/27/20 4:19 AM, Laurent Vivier wrote: > Le 24/07/2020 à 23:23, Richard Henderson a écrit : >> When the chroot does not have /proc mounted, we can read neither >> /proc/sys/vm/mmap_min_addr nor /proc/sys/maps. >> >> The enforcement of mmap_min_addr in the host kernel is done by >> the security module, and so does not apply to processes owned >> by root. Which leads pgd_find_hole_fallback to succeed in probing >> a reservation at address 0. Which confuses pgb_reserved_va to >> believe that guest_base has not actually been initialized. >> >> We don't actually want NULL addresses to become accessible, so >> make sure that mmap_min_addr is initialized with a non-zero value. >> >> Buglink: https://bugs.launchpad.net/qemu/+bug/1888728 >> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> linux-user/main.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> > > Richard, do you want to add this fix in one of your branches PR? I don't have anything queued at the moment. I thought I would be able to convince Alex to add it to his misc queue, but he sent that this morning. > In this case: > Acked-by: Laurent Vivier <laurent@vivier.eu> > > Otherwise I can prepare a small one for linux-user, adding one or two > fixes... Please go ahead. r~
Le 24/07/2020 à 23:23, Richard Henderson a écrit : > When the chroot does not have /proc mounted, we can read neither > /proc/sys/vm/mmap_min_addr nor /proc/sys/maps. > > The enforcement of mmap_min_addr in the host kernel is done by > the security module, and so does not apply to processes owned > by root. Which leads pgd_find_hole_fallback to succeed in probing > a reservation at address 0. Which confuses pgb_reserved_va to > believe that guest_base has not actually been initialized. > > We don't actually want NULL addresses to become accessible, so > make sure that mmap_min_addr is initialized with a non-zero value. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1888728 > Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/main.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/linux-user/main.c b/linux-user/main.c > index 3597e99bb1..75c9785157 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -758,14 +758,26 @@ int main(int argc, char **argv, char **envp) > > if ((fp = fopen("/proc/sys/vm/mmap_min_addr", "r")) != NULL) { > unsigned long tmp; > - if (fscanf(fp, "%lu", &tmp) == 1) { > + if (fscanf(fp, "%lu", &tmp) == 1 && tmp != 0) { > mmap_min_addr = tmp; > - qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n", mmap_min_addr); > + qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n", > + mmap_min_addr); > } > fclose(fp); > } > } > > + /* > + * We prefer to not make NULL pointers accessible to QEMU. > + * If we're in a chroot with no /proc, fall back to 1 page. > + */ > + if (mmap_min_addr == 0) { > + mmap_min_addr = qemu_host_page_size; > + qemu_log_mask(CPU_LOG_PAGE, > + "host mmap_min_addr=0x%lx (fallback)\n", > + mmap_min_addr); > + } > + > /* > * Prepare copy of argv vector for target. > */ > Applied to my linux-user-for-5.1 branch. Thanks, Laurent
diff --git a/linux-user/main.c b/linux-user/main.c index 3597e99bb1..75c9785157 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -758,14 +758,26 @@ int main(int argc, char **argv, char **envp) if ((fp = fopen("/proc/sys/vm/mmap_min_addr", "r")) != NULL) { unsigned long tmp; - if (fscanf(fp, "%lu", &tmp) == 1) { + if (fscanf(fp, "%lu", &tmp) == 1 && tmp != 0) { mmap_min_addr = tmp; - qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n", mmap_min_addr); + qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n", + mmap_min_addr); } fclose(fp); } } + /* + * We prefer to not make NULL pointers accessible to QEMU. + * If we're in a chroot with no /proc, fall back to 1 page. + */ + if (mmap_min_addr == 0) { + mmap_min_addr = qemu_host_page_size; + qemu_log_mask(CPU_LOG_PAGE, + "host mmap_min_addr=0x%lx (fallback)\n", + mmap_min_addr); + } + /* * Prepare copy of argv vector for target. */
When the chroot does not have /proc mounted, we can read neither /proc/sys/vm/mmap_min_addr nor /proc/sys/maps. The enforcement of mmap_min_addr in the host kernel is done by the security module, and so does not apply to processes owned by root. Which leads pgd_find_hole_fallback to succeed in probing a reservation at address 0. Which confuses pgb_reserved_va to believe that guest_base has not actually been initialized. We don't actually want NULL addresses to become accessible, so make sure that mmap_min_addr is initialized with a non-zero value. Buglink: https://bugs.launchpad.net/qemu/+bug/1888728 Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/main.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) -- 2.25.1