Re: [PATCH v3 3/5] selftests/x86: add shadow stack lock test

[email protected]
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The comment references a function name that does not exist in the code.
--

--- Patch [3]: [PATCH v3 3/5] selftests/x86: add shadow stack lock test ---
commit ff0ac7d8a7598f33a03169f21e6b92ce89cdf8a4
Author: Bill Roberts <[email protected]>

selftests/x86: add shadow stack lock test

This commit adds a test to lock the shadow stack write bit and verifies that
attempting to disable write fails with EPERM. It also ensures the write bit 
remains set for the whole test suite and preserves the ability to unlock it.

> diff --git a/tools/testing/selftests/x86/test_shadow_stack.c b/tools/testing/selftests/x86/test_shadow_stack.c
> index b52c5420c1374..72e7329d89d70 100644
> --- a/tools/testing/selftests/x86/test_shadow_stack.c
> +++ b/tools/testing/selftests/x86/test_shadow_stack.c

[ ... ]

> @@ -1079,12 +1097,33 @@ int main(int argc, char *argv[])
>  		return 1;
>  	}
>  
> +	/* Note: test_shadow_stack_lock() needs write enabled */

[Severity: Low]
This isn't a bug, but the comment refers to test_shadow_stack_lock() which
does not exist. Should this comment instead reference the test_locking()
function that is implemented and called in this patch?

> +	if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_WRSS)) {
> +		printf("[SKIP]\tCould not enable WRSS\n");
>  		ret = 1;
>  		goto out;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.