linux-kernel - Re: [PATCH v5 2/3] selftests/lam: Skip test if LAM is disabled

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: <81714cba-7924-4a4a-a5fc-b6f84bda352a@intel.com>
Date: Fri, 24 Jan 2025 08:23:09 -0800
From: Dave Hansen <dave.hansen@...el.com>
To: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>,
 shuah@...nel.org, hpa@...or.com, x86@...nel.org,
 dave.hansen@...ux.intel.com, bp@...en8.de, mingo@...hat.com,
 tglx@...utronix.de
Cc: linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
 kirill@...temov.name, "Kirill A. Shutemov"
 <kirill.shutemov@...ux.intel.com>, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH v5 2/3] selftests/lam: Skip test if LAM is disabled

On 11/27/24 09:35, Maciej Wieczor-Retman wrote:
> +static inline int kernel_has_lam(void)
> +{
> +	unsigned long bits;
> +
> +	syscall(SYS_arch_prctl, ARCH_GET_MAX_TAG_BITS, &bits);
> +	return !!bits;
> +}

Generally, I'm less picky about selftest/ code than in-kernel code. But
people really do take selftest code and use it as a starting point for
production code.

I'd much rather have overly verbose, obviously correct code:

	err = syscall(SYS_arch_prctl, ARCH_GET_MAX_TAG_BITS, &bits);

	/* Handle syscall failure, like pre-LAM kernels: */
	if (err)
		return 0

	/* Tag bits are empty on non-LAM systems: */
	return !!bits;

Actually, I was going to argue for that^ just on style and writing good
code. But then I spotted a bug. What happens if the kernel has
CONFIG_ADDRESS_MASKING=n, either because it is config'd off or it's old?
The:

	put_user(0, (unsigned long __user *)arg2);

won't ever get run and 'bits' will be uninitialized.

So, I think this code was trying to be compact, fast and clever. But it
really just turns out to be buggy.