Messages in this thread | | | Date | Thu, 5 May 2022 19:14:48 +0000 | From | Sean Christopherson <> | Subject | Re: [PATCH v7 10/11] KVM: selftests: Test disabling NX hugepages on a VM |
| |
On Tue, May 03, 2022, Ben Gardon wrote: > + if (disable_nx_huge_pages) { > + /* > + * Cannot run the test without NX huge pages if the kernel > + * does not support it. > + */ > + if (!kvm_check_cap(KVM_CAP_VM_DISABLE_NX_HUGE_PAGES)) > + return; > + > + r = __vm_disable_nx_huge_pages(vm); > + if (reboot_permissions) { > + TEST_ASSERT(!r, "Disabling NX huge pages should succeed if process has reboot permissions"); > + } else { > + TEST_ASSERT(r == -EPERM, "This process should not have permission to disable NX huge pages");
This is wrong, the return value on ioctl() failure is -1, the error code is in errno and it's a positive value.
LOL, but it passes because EPERM == 1, hilarious. To avoid confusion:
TEST_ASSERT(r == -1 && errno == EPERM, "This process should not have permission to disable NX huge pages");
> diff --git a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.sh b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.sh > index 60bfed8181b9..c21c1f639141 100755 > --- a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.sh > +++ b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.sh > @@ -16,6 +16,8 @@ HUGE_PAGES=$(sudo cat /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages) > > set +e > > +NXECUTABLE="$(dirname $0)/nx_huge_pages_test" > + > ( > set -e > > @@ -24,7 +26,15 @@ set +e > sudo echo 100 > /sys/module/kvm/parameters/nx_huge_pages_recovery_period_ms > sudo echo 3 > /sys/kernel/mm/hugepages/hugepages-2048kB/nr_hugepages > > - "$(dirname $0)"/nx_huge_pages_test 887563923 > + # Test with reboot permissions > + sudo setcap cap_sys_boot+ep $NXECUTABLE
This leaves cap_sys_boot set on the executable if the script is run as root.
Probably this? It's moderately user friendly without going too crazy on error handling.
# Test with reboot permissions if [ $(whoami) != "root" ] ; then sudo setcap cap_sys_boot+ep $NXECUTABLE fi $NXECUTABLE 887563923 1
# Test without reboot permissions if [ $(whoami) != "root" ] ; then sudo setcap cap_sys_boot-ep $NXECUTABLE $NXECUTABLE 887563923 0 fi
> + $NXECUTABLE 887563923 1 > + > + # Test without reboot permissions > + if [ $(whoami) != "root" ] ; then > + sudo setcap cap_sys_boot-ep $NXECUTABLE > + $NXECUTABLE 887563923 0
I would much prefer a proper flag, not a magic 0 vs. 1.
> + fi > ) > RET=$? > > -- > 2.36.0.464.gb9c8b46e94-goog >
| |