| Message ID | 20250113-virtual_address_range-tests-v3-4-f4a8e6b7feed@linutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/mm: virtual_address_range: Reduce memory usage and avoid VM_IO access | expand |
On Mon, 13 Jan 2025 14:15:38 +0100 Thomas Weißschuh <thomas.weissschuh@linutronix.de> wrote: > The virtual_address_range selftest reads from the start of each mapping > listed in /proc/self/maps. > However not all mappings are valid to be arbitrarily accessed. > > For example the vvar data used for virtual clocks on x86 [vvar_vclock] > can only be accessed if 1) the kernel configuration enables virtual > clocks and 2) the hypervisor provided the data for it. > Only the VDSO itself has the necessary information to know this. > Since commit e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") > the virtual clock data was split out into its own mapping, leading > to EFAULT from read() during the validation. > > Check for the VM_IO flag as a proxy. > It is present for the VVAR mappings and MMIO ranges can be dangerous to > access arbitrarily. > > Reported-by: kernel test robot <oliver.sang@intel.com> > Closes: https://lore.kernel.org/oe-lkp/202412271148.2656e485-lkp@intel.com > Fixes: e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") > Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without reliance on correctness of mmap()") > Suggested-by: David Hildenbrand <david@redhat.com> > Link: https://lore.kernel.org/lkml/e97c2a5d-c815-4936-a767-ac42a3220a90@redhat.com/ > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > Revert "selftests/mm: virtual_address_range: Avoid reading VVAR mappings" > > This reverts commit 05cc5d292ac4238684b59922aecf59c932edefa0. I'm assuming that the above two lines simply shouldn't be here?
On 13.01.25 14:15, Thomas Weißschuh wrote: > The virtual_address_range selftest reads from the start of each mapping > listed in /proc/self/maps. > However not all mappings are valid to be arbitrarily accessed. > > For example the vvar data used for virtual clocks on x86 [vvar_vclock] > can only be accessed if 1) the kernel configuration enables virtual > clocks and 2) the hypervisor provided the data for it. > Only the VDSO itself has the necessary information to know this. > Since commit e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") > the virtual clock data was split out into its own mapping, leading > to EFAULT from read() during the validation. > > Check for the VM_IO flag as a proxy. > It is present for the VVAR mappings and MMIO ranges can be dangerous to > access arbitrarily. > > Reported-by: kernel test robot <oliver.sang@intel.com> > Closes: https://lore.kernel.org/oe-lkp/202412271148.2656e485-lkp@intel.com > Fixes: e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") > Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without reliance on correctness of mmap()") > Suggested-by: David Hildenbrand <david@redhat.com> > Link: https://lore.kernel.org/lkml/e97c2a5d-c815-4936-a767-ac42a3220a90@redhat.com/ > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > Revert "selftests/mm: virtual_address_range: Avoid reading VVAR mappings" > > This reverts commit 05cc5d292ac4238684b59922aecf59c932edefa0. > --- > tools/testing/selftests/mm/virtual_address_range.c | 4 ++++ > tools/testing/selftests/mm/vm_util.c | 21 +++++++++++++++++++++ > tools/testing/selftests/mm/vm_util.h | 1 + > 3 files changed, 26 insertions(+) > > diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c > index 386e4e46fa65b98af78dee4bb30144eb2b51f528..b380e102b22f0a44654ab046f257e8c35e8d90e9 100644 > --- a/tools/testing/selftests/mm/virtual_address_range.c > +++ b/tools/testing/selftests/mm/virtual_address_range.c > @@ -15,6 +15,7 @@ > #include <sys/time.h> > #include <fcntl.h> > > +#include "vm_util.h" > #include "../kselftest.h" > > /* > @@ -159,6 +160,9 @@ static int validate_complete_va_space(void) > if (prot[0] != 'r') > continue; > > + if (check_vmflag_io((void *)start_addr)) > + continue; > + > /* > * Confirm whether MAP_CHUNK_SIZE chunk can be found or not. > * If write succeeds, no need to check MAP_CHUNK_SIZE - 1 > diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c > index 8468a19d6acca10c7e9228c03a935cdeb2402b5d..161fe03b07af78244efc669a36155ad603fa6f7d 100644 > --- a/tools/testing/selftests/mm/vm_util.c > +++ b/tools/testing/selftests/mm/vm_util.c > @@ -400,3 +400,24 @@ unsigned long get_free_hugepages(void) > fclose(f); > return fhp; > } > + > +bool check_vmflag_io(void *addr) Interestingly, this will only work if addr really is the start of the mapping, as __get_smap_entry() cannot handle "address falls into a mapping". Good enough for now, but might be worth a comment. (wrong usage would fail with No VmFlags for ... ) > +{ > + char *saveptr, *flag, *strtok_arg; " On some implementations, *saveptr is required to be NULL on the first call to strtok_r() that is being used to parse str." Maybe just initialize it to NULL. > + char buffer[MAX_LINE_LENGTH]; > + > + strtok_arg = __get_smap_entry(addr, "VmFlags:", buffer, sizeof(buffer)); > + if (!strtok_arg) > + ksft_exit_fail_msg("%s: No VmFlags for %p\n", __func__, addr); > + > + while (true) { > + flag = strtok_r(strtok_arg, " ", &saveptr); > + if (!flag) > + break; > + if (strcmp(flag, "io") == 0) > + return true; Are these early exit allowed with strtok_r()? An alternative seems to be using strcspn() that doesn't modify strings/maintain state in-between calls.
diff --git a/tools/testing/selftests/mm/virtual_address_range.c b/tools/testing/selftests/mm/virtual_address_range.c index 386e4e46fa65b98af78dee4bb30144eb2b51f528..b380e102b22f0a44654ab046f257e8c35e8d90e9 100644 --- a/tools/testing/selftests/mm/virtual_address_range.c +++ b/tools/testing/selftests/mm/virtual_address_range.c @@ -15,6 +15,7 @@ #include <sys/time.h> #include <fcntl.h> +#include "vm_util.h" #include "../kselftest.h" /* @@ -159,6 +160,9 @@ static int validate_complete_va_space(void) if (prot[0] != 'r') continue; + if (check_vmflag_io((void *)start_addr)) + continue; + /* * Confirm whether MAP_CHUNK_SIZE chunk can be found or not. * If write succeeds, no need to check MAP_CHUNK_SIZE - 1 diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c index 8468a19d6acca10c7e9228c03a935cdeb2402b5d..161fe03b07af78244efc669a36155ad603fa6f7d 100644 --- a/tools/testing/selftests/mm/vm_util.c +++ b/tools/testing/selftests/mm/vm_util.c @@ -400,3 +400,24 @@ unsigned long get_free_hugepages(void) fclose(f); return fhp; } + +bool check_vmflag_io(void *addr) +{ + char *saveptr, *flag, *strtok_arg; + char buffer[MAX_LINE_LENGTH]; + + strtok_arg = __get_smap_entry(addr, "VmFlags:", buffer, sizeof(buffer)); + if (!strtok_arg) + ksft_exit_fail_msg("%s: No VmFlags for %p\n", __func__, addr); + + while (true) { + flag = strtok_r(strtok_arg, " ", &saveptr); + if (!flag) + break; + if (strcmp(flag, "io") == 0) + return true; + strtok_arg = NULL; + } + + return false; +} diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index 2eaed82099255e09ffd38ad9714994397f304685..b60ac68a9dc8893895f49946b258260f7a82218a 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -53,6 +53,7 @@ int uffd_unregister(int uffd, void *addr, uint64_t len); int uffd_register_with_ioctls(int uffd, void *addr, uint64_t len, bool miss, bool wp, bool minor, uint64_t *ioctls); unsigned long get_free_hugepages(void); +bool check_vmflag_io(void *addr); /* * On ppc64 this will only work with radix 2M hugepage size
The virtual_address_range selftest reads from the start of each mapping listed in /proc/self/maps. However not all mappings are valid to be arbitrarily accessed. For example the vvar data used for virtual clocks on x86 [vvar_vclock] can only be accessed if 1) the kernel configuration enables virtual clocks and 2) the hypervisor provided the data for it. Only the VDSO itself has the necessary information to know this. Since commit e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") the virtual clock data was split out into its own mapping, leading to EFAULT from read() during the validation. Check for the VM_IO flag as a proxy. It is present for the VVAR mappings and MMIO ranges can be dangerous to access arbitrarily. Reported-by: kernel test robot <oliver.sang@intel.com> Closes: https://lore.kernel.org/oe-lkp/202412271148.2656e485-lkp@intel.com Fixes: e93d2521b27f ("x86/vdso: Split virtual clock pages into dedicated mapping") Fixes: 010409649885 ("selftests/mm: confirm VA exhaustion without reliance on correctness of mmap()") Suggested-by: David Hildenbrand <david@redhat.com> Link: https://lore.kernel.org/lkml/e97c2a5d-c815-4936-a767-ac42a3220a90@redhat.com/ Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> Revert "selftests/mm: virtual_address_range: Avoid reading VVAR mappings" This reverts commit 05cc5d292ac4238684b59922aecf59c932edefa0. --- tools/testing/selftests/mm/virtual_address_range.c | 4 ++++ tools/testing/selftests/mm/vm_util.c | 21 +++++++++++++++++++++ tools/testing/selftests/mm/vm_util.h | 1 + 3 files changed, 26 insertions(+)