Re: [PATCH] riscv: selftests: Fix pointer masking test failures on real hardware

[Samuel Holland]

Hi,

On 2026-07-21 1:52 AM, JinRui wrote:
> From: jinrui <jinrui@xxxxxxxxxxx>
> 
> The pointer masking selftest contains several bugs that cause it to fail
> on real RISC-V hardware:
> 
> 1. test_pmlen() did not pass PR_TAGGED_ADDR_ENABLE when requesting a
>    non-zero PMLEN. The kernel requires this flag; without it, PMLEN is
>    forced to 0 regardless of the requested value (see process.c:352).

Ouch, sorry, this is a bug in the kernel, introduced by commit 3033b2b1e394
("riscv: Reset pmm when PR_TAGGED_ADDR_ENABLE is not set"). The kernel
documentation and existing selftest logic are correct. PMLEN is independent from
PR_TAGGED_ADDR_ENABLE so userspace can use pointer masking internally without
changing the syscall ABI. This allows userspace software to behave like it does
on arm64, where TBI is always enabled, but the tagged address ABI is disabled by
default.

I was not paying enough attention and should not have Reviewed-by that patch. I
just sent a revert:
https://lore.kernel.org/linux-riscv/20260820014551.1979772-1-samuel.holland@xxxxxxxxxx/

> 2. set_tagged_addr_ctrl() required the PR_GET return value to exactly
>    match the PR_SET argument. However, the kernel may legitimately:
>    - Force PMLEN to 0 when the ABI is disabled (tagged_addr_abi=false)
>    - Round up PMLEN (e.g., request=1 -> PMLEN=7 or PMLEN=16)

There should be no rounding here, because set_tagged_addr_ctrl() is only ever
called with pmlen set to 0, min_pmlen, or max_pmlen. All of these are values
that the kernel is known to support (in other words, they are already rounded).
The remaining issues described here should be fixed by the revert linked above.

Regards,
Samuel

> 3. test_dereference_pmlen() always passed tagged_addr_abi=false, even
>    for non-zero PMLEN values. This meant PMLEN was never actually
>    enabled, so the valid tag dereference test always triggered SIGSEGV.
> 
> 4. test_fork_exec() passed tagged_addr_abi=false, causing the fork
>    child's tagged pointer dereference to fail because the inherited
>    PMLEN was 0.
> 
> Additionally, add an explicit #include <stdint.h> for uintptr_t, which
> is required by the C standard and must be included explicitly on some
> compiler/libc combinations.
> 
> Fixes: 7470b5afd150 ("riscv: selftests: Add a pointer masking test")
> Signed-off-by: jinrui <jinrui@xxxxxxxxxxx>
> ---
>  .../selftests/riscv/abi/pointer_masking.c     | 34 +++++++++++++++++--
>  1 file changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/riscv/abi/pointer_masking.c b/tools/testing/selftests/riscv/abi/pointer_masking.c
> index 2d540af7b558..106f12f1ea44 100644
> --- a/tools/testing/selftests/riscv/abi/pointer_masking.c
> +++ b/tools/testing/selftests/riscv/abi/pointer_masking.c
> @@ -5,6 +5,7 @@
>  #include <setjmp.h>
>  #include <signal.h>
>  #include <stdbool.h>
> +#include <stdint.h>
>  #include <sys/prctl.h>
>  #include <sys/wait.h>
>  #include <unistd.h>
> @@ -17,6 +18,9 @@
>  #ifndef PR_PMLEN_MASK
>  #define PR_PMLEN_MASK			(0x7fUL << PR_PMLEN_SHIFT)
>  #endif
> +#ifndef PR_TAGGED_ADDR_ENABLE
> +#define PR_TAGGED_ADDR_ENABLE		1
> +#endif
>  
>  static int dev_zero;
>  
> @@ -44,7 +48,10 @@ static void test_pmlen(void)
>  	for (int request = 0; request <= 16; request++) {
>  		int pmlen, ret;
>  
> -		ret = prctl(PR_SET_TAGGED_ADDR_CTRL, request << PR_PMLEN_SHIFT, 0, 0, 0);
> +		ret = prctl(PR_SET_TAGGED_ADDR_CTRL,
> +			    request << PR_PMLEN_SHIFT |
> +			    (request ? PR_TAGGED_ADDR_ENABLE : 0),
> +			    0, 0, 0);
>  		if (ret)
>  			goto pr_set_error;
>  
> @@ -85,6 +92,22 @@ static int set_tagged_addr_ctrl(int pmlen, bool tagged_addr_abi)
>  		ret = prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0);
>  		if (ret == arg)
>  			return 0;
> +
> +		/*
> +		 * When tagged_addr_abi is false, the kernel forces PMLEN
> +		 * to 0 regardless of what was requested. Accept the result
> +		 * as long as PR_TAGGED_ADDR_ENABLE is not set.
> +		 */
> +		if (!tagged_addr_abi && !(ret & PR_TAGGED_ADDR_ENABLE))
> +			return 0;
> +
> +		/*
> +		 * When tagged_addr_abi is true, the kernel may round up
> +		 * the PMLEN (e.g. request=1 -> PMLEN=7 or PMLEN=16).
> +		 * Accept the result if the enable bit is set.
> +		 */
> +		if (tagged_addr_abi && (ret & PR_TAGGED_ADDR_ENABLE))
> +			return 0;
>  	}
>  
>  	return ret < 0 ? -errno : -ENODATA;
> @@ -96,7 +119,12 @@ static void test_dereference_pmlen(int pmlen)
>  	volatile int *p;
>  	int ret;
>  
> -	ret = set_tagged_addr_ctrl(pmlen, false);
> +	/*
> +	 * When pmlen is 0, the tagged address ABI cannot be enabled (the
> +	 * kernel rejects PR_TAGGED_ADDR_ENABLE when PMLEN is 0). Disable
> +	 * pointer masking and test the invalid tag path instead.
> +	 */
> +	ret = set_tagged_addr_ctrl(pmlen, pmlen ? true : false);
>  	if (ret)
>  		return ksft_test_result_error("PMLEN=%d setup (%d)\n", pmlen, ret);
>  
> @@ -157,7 +185,7 @@ static void test_fork_exec(void)
>  
>  	ksft_print_msg("Testing fork/exec behavior\n");
>  
> -	ret = set_tagged_addr_ctrl(min_pmlen, false);
> +	ret = set_tagged_addr_ctrl(min_pmlen, true);
>  	if (ret)
>  		return ksft_test_result_error("setup (%d)\n", ret);
>