linux-kernel - Re: [PATCH] selftests/mm: Don't needlessly use sudo to obtain root in run

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <17c0b7a1-6ec2-4504-8287-f0fa111b9748@arm.com>
Date: Sat, 10 Feb 2024 07:40:16 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: Mark Brown <broonie@...nel.org>, Andrew Morton
 <akpm@...ux-foundation.org>, Shuah Khan <shuah@...nel.org>
Cc: linux-mm@...ck.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] selftests/mm: Don't needlessly use sudo to obtain root in
 run_vmtests.sh

On 09/02/2024 20:21, Mark Brown wrote:
> When opening yama/ptrace_scope we unconditionally use sudo to ensure we
> are running as root, resulting in failures if running in a minimal root
> filesystem where sudo is not installed. Since automated test systems will
> typically just run all of kselftest as root (and many kselftests rely on
> this for full functionality) add a check to see if we're already root and
> only invoke sudo if not.

I don't really see the point of this. run_vmtests.sh needs to be run as root;
there are lots of operations that depend on it and most tests will fail if not
root. So I think it would be much cleaner just to remove this instance sudo.

The problem that I was referring to yesterday, about needing sudo was for this case:

CATEGORY="mlock" run_test sudo -u nobody ./on-fault-limit

Here, we are using sudo to deprivilege ourselves from root and run
on-fault-limit as nobody. This is required because the test is checking an
rlimit that is only enforced for normal users.

Somebody on list was talking about skipping this test if sudo wasn't present a
couple of weeks back. Not sure if that happened.

> 
> Since I am unclear what the intended effect of the command being run is I
> have not added any error handling for the case where we fail to obtain
> root.
> 
> Signed-off-by: Mark Brown <broonie@...nel.org>
> ---
>  tools/testing/selftests/mm/run_vmtests.sh | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh
> index fe140a9f4f9d..c8ca830dba93 100755
> --- a/tools/testing/selftests/mm/run_vmtests.sh
> +++ b/tools/testing/selftests/mm/run_vmtests.sh
> @@ -248,6 +248,17 @@ run_test() {
>  
>  echo "TAP version 13" | tap_output
>  
> +HAVE_ROOT=0
> +if [ "$(id -u)" = "0" ]; then
> +	AS_ROOT=
> +	HAVE_ROOT=1
> +elif [ "$(command -v sudo)" != "" ]; then
> +	AS_ROOT=sudo
> +	HAVE_ROOT=1
> +else
> +	echo # WARNING: Unable to run as root
> +fi
> +
>  CATEGORY="hugetlb" run_test ./hugepage-mmap
>  
>  shmmax=$(cat /proc/sys/kernel/shmmax)
> @@ -363,7 +374,8 @@ CATEGORY="hmm" run_test bash ./test_hmm.sh smoke
>  # MADV_POPULATE_READ and MADV_POPULATE_WRITE tests
>  CATEGORY="madv_populate" run_test ./madv_populate
>  
> -(echo 0 | sudo tee /proc/sys/kernel/yama/ptrace_scope 2>&1) | tap_prefix
> +# FIXME: What if we can't get root?
> +(echo 0 | ${AS_ROOT} tee /proc/sys/kernel/yama/ptrace_scope 2>&1) | tap_prefix
>  CATEGORY="memfd_secret" run_test ./memfd_secret
>  
>  # KSM KSM_MERGE_TIME_HUGE_PAGES test with size of 100
> 
> ---
> base-commit: 445a555e0623387fa9b94e68e61681717e70200a
> change-id: 20240209-kselftest-mm-check-deps-01a825e5fed4
> 
> Best regards,