[PATCH v2 0/6] add more tests to MTE kselftests

[PATCH v2 0/6] add more tests to MTE kselftests

[Joey Gouly]

This is a series which refactors and then adds some extra tests for MTE
in the kselftest framework.

The issue that these tests are for was was fixed by Robin in
295cf156231c ("arm64: Avoid premature usercopy failure") and based on a
simplified example by Catalin [1].

They test some combinations of pointer offsets, sizes and syscalls to
exercise different paths in the kernel.

Changes since v1 [2]:
  - add a page_sz to the tests, for non-4K kernels
  - use loops to generate more tests and avoid long lines

Thanks,
Joey

[1] https://lore.kernel.org/all/20210624150911.GA25097@arm.com/
[2] https://lore.kernel.org/linux-arm-kernel/20220125150920.23955-1-joey.gouly@arm.com/

Joey Gouly (6):
  kselftest/arm64: mte: user_mem: introduce tag_offset and tag_len
  kselftest/arm64: mte: user_mem: rework error handling
  kselftest/arm64: mte: user_mem: check different offsets and sizes
  kselftest/arm64: mte: user_mem: add test type enum
  kselftest/arm64: mte: user_mem: add more test types
  kselftest/arm64: mte: user_mem: test more combinations of values

 .../selftests/arm64/mte/check_user_mem.c      | 196 +++++++++++++++---
 1 file changed, 162 insertions(+), 34 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/6] kselftest/arm64: mte: user_mem: introduce tag_offset and tag_len

[Joey Gouly]

These can be used to place a different tag not at a page size boundary.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
---
 .../selftests/arm64/mte/check_user_mem.c      | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
index 1de7a0abd0ae..5a5a7e1f5789 100644
--- a/tools/testing/selftests/arm64/mte/check_user_mem.c
+++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
@@ -19,7 +19,8 @@
 
 static size_t page_sz;
 
-static int check_usermem_access_fault(int mem_type, int mode, int mapping)
+static int check_usermem_access_fault(int mem_type, int mode, int mapping,
+                                      int tag_offset, int tag_len)
 {
 	int fd, i, err;
 	char val = 'A';
@@ -54,10 +55,12 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping)
 	if (i < len)
 		goto usermem_acc_err;
 
-	/* Tag the next half of memory with different value */
-	ptr_next = (void *)((unsigned long)ptr + page_sz);
+	if (!tag_len)
+		tag_len = len - tag_offset;
+	/* Tag a part of memory with different value */
+	ptr_next = (void *)((unsigned long)ptr + tag_offset);
 	ptr_next = mte_insert_new_tag(ptr_next);
-	mte_set_tag_address_range(ptr_next, page_sz);
+	mte_set_tag_address_range(ptr_next, tag_len);
 
 	lseek(fd, 0, 0);
 	/* Copy from file into buffer with invalid tag */
@@ -100,14 +103,14 @@ int main(int argc, char *argv[])
 	/* Set test plan */
 	ksft_set_plan(4);
 
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE),
+	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE, page_sz, 0),
 		"Check memory access from kernel in sync mode, private mapping and mmap memory\n");
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED),
+	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED, page_sz, 0),
 		"Check memory access from kernel in sync mode, shared mapping and mmap memory\n");
 
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE),
+	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE, page_sz, 0),
 		"Check memory access from kernel in async mode, private mapping and mmap memory\n");
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED),
+	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED, page_sz, 0),
 		"Check memory access from kernel in async mode, shared mapping and mmap memory\n");
 
 	mte_restore_setup();
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/6] kselftest/arm64: mte: user_mem: introduce tag_offset and tag_len

[Shuah Khan]

On 2/3/22 7:36 AM, Joey Gouly wrote:
> These can be used to place a different tag not at a page size boundary.

Please add more information here. Based on the code instead of pagesz, this
test allows other sizes. This is a good change and bit more info. helps
describe the value of this change.

> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>   .../selftests/arm64/mte/check_user_mem.c      | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
> index 1de7a0abd0ae..5a5a7e1f5789 100644
> --- a/tools/testing/selftests/arm64/mte/check_user_mem.c
> +++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
> @@ -19,7 +19,8 @@
>   
>   static size_t page_sz;
>   
> -static int check_usermem_access_fault(int mem_type, int mode, int mapping)
> +static int check_usermem_access_fault(int mem_type, int mode, int mapping,
> +                                      int tag_offset, int tag_len)
>   {
>   	int fd, i, err;
>   	char val = 'A';
> @@ -54,10 +55,12 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping)
>   	if (i < len)
>   		goto usermem_acc_err;
>   
> -	/* Tag the next half of memory with different value */
> -	ptr_next = (void *)((unsigned long)ptr + page_sz);
> +	if (!tag_len)
> +		tag_len = len - tag_offset;
> +	/* Tag a part of memory with different value */
> +	ptr_next = (void *)((unsigned long)ptr + tag_offset);
>   	ptr_next = mte_insert_new_tag(ptr_next);
> -	mte_set_tag_address_range(ptr_next, page_sz);
> +	mte_set_tag_address_range(ptr_next, tag_len);
>   
>   	lseek(fd, 0, 0);
>   	/* Copy from file into buffer with invalid tag */
> @@ -100,14 +103,14 @@ int main(int argc, char *argv[])
>   	/* Set test plan */
>   	ksft_set_plan(4);
>   
> -	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE),
> +	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE, page_sz, 0),
>   		"Check memory access from kernel in sync mode, private mapping and mmap memory\n");

Any reason to not print the size - old message is better?

> -	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED),
> +	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED, page_sz, 0),
>   		"Check memory access from kernel in sync mode, shared mapping and mmap memory\n");
>   
> -	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE),
> +	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE, page_sz, 0),
>   		"Check memory access from kernel in async mode, private mapping and mmap memory\n");
> -	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED),
> +	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED, page_sz, 0),
>   		"Check memory access from kernel in async mode, shared mapping and mmap memory\n");
>   
>   	mte_restore_setup();
> 

With these comments:

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/6] kselftest/arm64: mte: user_mem: rework error handling

[Joey Gouly]

This makes it easier to have multiple iterations in the test.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
---
 .../selftests/arm64/mte/check_user_mem.c      | 27 ++++++++++++-------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
index 5a5a7e1f5789..e5bfeaeb64bb 100644
--- a/tools/testing/selftests/arm64/mte/check_user_mem.c
+++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
@@ -27,7 +27,7 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 	size_t len, read_len;
 	void *ptr, *ptr_next;
 
-	err = KSFT_FAIL;
+	err = KSFT_PASS;
 	len = 2 * page_sz;
 	mte_switch_mode(mode, MTE_ALLOW_NON_ZERO_TAG);
 	fd = create_temp_file();
@@ -46,14 +46,18 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 	/* Copy from file into buffer with valid tag */
 	read_len = read(fd, ptr, len);
 	mte_wait_after_trig();
-	if (cur_mte_cxt.fault_valid || read_len < len)
+	if (cur_mte_cxt.fault_valid || read_len < len) {
+		err = KSFT_FAIL;
 		goto usermem_acc_err;
+	}
 	/* Verify same pattern is read */
 	for (i = 0; i < len; i++)
 		if (*(char *)(ptr + i) != val)
 			break;
-	if (i < len)
+	if (i < len) {
+		err = KSFT_FAIL;
 		goto usermem_acc_err;
+	}
 
 	if (!tag_len)
 		tag_len = len - tag_offset;
@@ -71,12 +75,17 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 	 * mode without fault but may not fail in async mode as per the
 	 * implemented MTE userspace support in Arm64 kernel.
 	 */
-	if (mode == MTE_SYNC_ERR &&
-	    !cur_mte_cxt.fault_valid && read_len < len) {
-		err = KSFT_PASS;
-	} else if (mode == MTE_ASYNC_ERR &&
-		   !cur_mte_cxt.fault_valid && read_len == len) {
-		err = KSFT_PASS;
+	if (cur_mte_cxt.fault_valid) {
+		err = KSFT_FAIL;
+		goto usermem_acc_err;
+	}
+	if (mode == MTE_SYNC_ERR && read_len < len) {
+		/* test passed */
+	} else if (mode == MTE_ASYNC_ERR && read_len == len) {
+		/* test passed */
+	} else {
+		err = KSFT_FAIL;
+		goto usermem_acc_err;
 	}
 usermem_acc_err:
 	mte_free_memory((void *)ptr, len, mem_type, true);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 2/6] kselftest/arm64: mte: user_mem: rework error handling

[Shuah Khan]

On 2/3/22 7:36 AM, Joey Gouly wrote:
> This makes it easier to have multiple iterations in the test.

Please add more information here. Based on the code instead of pagesz, this
test allows other sizes. This is a good change and bit more info. helps
describe the value of this change.

> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>   .../selftests/arm64/mte/check_user_mem.c      | 27 ++++++++++++-------
>   1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
> index 5a5a7e1f5789..e5bfeaeb64bb 100644
> --- a/tools/testing/selftests/arm64/mte/check_user_mem.c
> +++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
> @@ -27,7 +27,7 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
>   	size_t len, read_len;
>   	void *ptr, *ptr_next;
>   
> -	err = KSFT_FAIL;
> +	err = KSFT_PASS;
>   	len = 2 * page_sz;
>   	mte_switch_mode(mode, MTE_ALLOW_NON_ZERO_TAG);
>   	fd = create_temp_file();
> @@ -46,14 +46,18 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
>   	/* Copy from file into buffer with valid tag */
>   	read_len = read(fd, ptr, len);
>   	mte_wait_after_trig();
> -	if (cur_mte_cxt.fault_valid || read_len < len)
> +	if (cur_mte_cxt.fault_valid || read_len < len) {
> +		err = KSFT_FAIL;

Why not set err in usermem_acc_err - this way you can make the error
paths common and simpler.

>   		goto usermem_acc_err;
> +	}
>   	/* Verify same pattern is read */
>   	for (i = 0; i < len; i++)
>   		if (*(char *)(ptr + i) != val)
>   			break;
> -	if (i < len)
> +	if (i < len) {
> +		err = KSFT_FAIL;

The above assignment can go way if you add this to usermem_acc_err
path

>   		goto usermem_acc_err;
> +	}
>   
>   	if (!tag_len)
>   		tag_len = len - tag_offset;
> @@ -71,12 +75,17 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
>   	 * mode without fault but may not fail in async mode as per the
>   	 * implemented MTE userspace support in Arm64 kernel.
>   	 */
> -	if (mode == MTE_SYNC_ERR &&
> -	    !cur_mte_cxt.fault_valid && read_len < len) {
> -		err = KSFT_PASS;
> -	} else if (mode == MTE_ASYNC_ERR &&
> -		   !cur_mte_cxt.fault_valid && read_len == len) {
> -		err = KSFT_PASS;
> +	if (cur_mte_cxt.fault_valid) {
> +		err = KSFT_FAIL;

The above assignment can go way if you add this to usermem_acc_err
path

> +		goto usermem_acc_err;
> +	}
> +	if (mode == MTE_SYNC_ERR && read_len < len) {
> +		/* test passed */
> +	} else if (mode == MTE_ASYNC_ERR && read_len == len) {
> +		/* test passed */
> +	} else {
> +		err = KSFT_FAIL;

The above assignment can go way if you add this to usermem_acc_err
path

> +		goto usermem_acc_err;
>   	}
>   usermem_acc_err:
>   	mte_free_memory((void *)ptr, len, mem_type, true);
> 

With the above requested changes:

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/6] kselftest/arm64: mte: user_mem: check different offsets and sizes

[Joey Gouly]

Iterate over different file offsets, pointer offsets and buffer sizes to exercise
and test different behaviour.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
---
 .../selftests/arm64/mte/check_user_mem.c      | 49 +++++++++++--------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
index e5bfeaeb64bb..35061ae23ed5 100644
--- a/tools/testing/selftests/arm64/mte/check_user_mem.c
+++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
@@ -26,6 +26,8 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 	char val = 'A';
 	size_t len, read_len;
 	void *ptr, *ptr_next;
+	int fileoff, ptroff, size;
+	int sizes[] = {1, 2, 3, 8, 16, 32, 4096, page_sz};
 
 	err = KSFT_PASS;
 	len = 2 * page_sz;
@@ -66,26 +68,33 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 	ptr_next = mte_insert_new_tag(ptr_next);
 	mte_set_tag_address_range(ptr_next, tag_len);
 
-	lseek(fd, 0, 0);
-	/* Copy from file into buffer with invalid tag */
-	read_len = read(fd, ptr, len);
-	mte_wait_after_trig();
-	/*
-	 * Accessing user memory in kernel with invalid tag should fail in sync
-	 * mode without fault but may not fail in async mode as per the
-	 * implemented MTE userspace support in Arm64 kernel.
-	 */
-	if (cur_mte_cxt.fault_valid) {
-		err = KSFT_FAIL;
-		goto usermem_acc_err;
-	}
-	if (mode == MTE_SYNC_ERR && read_len < len) {
-		/* test passed */
-	} else if (mode == MTE_ASYNC_ERR && read_len == len) {
-		/* test passed */
-	} else {
-		err = KSFT_FAIL;
-		goto usermem_acc_err;
+	for (fileoff = 0; fileoff < 16; fileoff++) {
+		for (ptroff = 0; ptroff < 16; ptroff++) {
+			for (i = 0; i < ARRAY_SIZE(sizes); i++) {
+				size = sizes[i];
+				lseek(fd, 0, 0);
+				/* Copy from file into buffer with invalid tag */
+				read_len = read(fd, ptr + ptroff, size);
+				mte_wait_after_trig();
+				/*
+				 * Accessing user memory in kernel with invalid tag should fail in sync
+				 * mode without fault but may not fail in async mode as per the
+				 * implemented MTE userspace support in Arm64 kernel.
+				 */
+				if (cur_mte_cxt.fault_valid) {
+					err = KSFT_FAIL;
+					goto usermem_acc_err;
+				}
+				if (mode == MTE_SYNC_ERR && read_len < len) {
+					/* test passed */
+				} else if (mode == MTE_ASYNC_ERR && read_len == size) {
+					/* test passed */
+				} else {
+					err = KSFT_FAIL;
+					goto usermem_acc_err;
+				}
+			}
+		}
 	}
 usermem_acc_err:
 	mte_free_memory((void *)ptr, len, mem_type, true);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/6] kselftest/arm64: mte: user_mem: check different offsets and sizes

[Shuah Khan]

On 2/3/22 7:36 AM, Joey Gouly wrote:
> Iterate over different file offsets, pointer offsets and buffer sizes to exercise
> and test different behaviour.

Please add more information here.

> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>   .../selftests/arm64/mte/check_user_mem.c      | 49 +++++++++++--------
>   1 file changed, 29 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
> index e5bfeaeb64bb..35061ae23ed5 100644
> --- a/tools/testing/selftests/arm64/mte/check_user_mem.c
> +++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
> @@ -26,6 +26,8 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
>   	char val = 'A';
>   	size_t len, read_len;
>   	void *ptr, *ptr_next;
> +	int fileoff, ptroff, size;
> +	int sizes[] = {1, 2, 3, 8, 16, 32, 4096, page_sz};
>   
>   	err = KSFT_PASS;
>   	len = 2 * page_sz;
> @@ -66,26 +68,33 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
>   	ptr_next = mte_insert_new_tag(ptr_next);
>   	mte_set_tag_address_range(ptr_next, tag_len);
>   
> -	lseek(fd, 0, 0);
> -	/* Copy from file into buffer with invalid tag */
> -	read_len = read(fd, ptr, len);
> -	mte_wait_after_trig();
> -	/*
> -	 * Accessing user memory in kernel with invalid tag should fail in sync
> -	 * mode without fault but may not fail in async mode as per the
> -	 * implemented MTE userspace support in Arm64 kernel.
> -	 */
> -	if (cur_mte_cxt.fault_valid) {
> -		err = KSFT_FAIL;


> -		goto usermem_acc_err;
> -	}
> -	if (mode == MTE_SYNC_ERR && read_len < len) {
> -		/* test passed */
> -	} else if (mode == MTE_ASYNC_ERR && read_len == len) {
> -		/* test passed */
> -	} else {
> -		err = KSFT_FAIL;

Why not set err in usermem_acc_err - this way you can make the error
paths common and simpler.

> -		goto usermem_acc_err;
> +	for (fileoff = 0; fileoff < 16; fileoff++) {
> +		for (ptroff = 0; ptroff < 16; ptroff++) {
> +			for (i = 0; i < ARRAY_SIZE(sizes); i++) {
> +				size = sizes[i];
> +				lseek(fd, 0, 0);
> +				/* Copy from file into buffer with invalid tag */
> +				read_len = read(fd, ptr + ptroff, size);
> +				mte_wait_after_trig();
> +				/*
> +				 * Accessing user memory in kernel with invalid tag should fail in sync
> +				 * mode without fault but may not fail in async mode as per the
> +				 * implemented MTE userspace support in Arm64 kernel.
> +				 */
> +				if (cur_mte_cxt.fault_valid) {
> +					err = KSFT_FAIL;

The above assignment can go way if you add this to usermem_acc_err
path

> +					goto usermem_acc_err;
> +				}
> +				if (mode == MTE_SYNC_ERR && read_len < len) {
> +					/* test passed */
> +				} else if (mode == MTE_ASYNC_ERR && read_len == size) {
> +					/* test passed */
> +				} else {
> +					err = KSFT_FAIL;

Same here

> +					goto usermem_acc_err;
> +				}
> +			}
> +		}
>   	}
>   usermem_acc_err:
>   	mte_free_memory((void *)ptr, len, mem_type, true);
> 

With the above requested changes -

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/6] kselftest/arm64: mte: user_mem: add test type enum

[Joey Gouly]

This enum will be used to parameterise the test to perform other syscalls
than 'read'.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
---
 .../selftests/arm64/mte/check_user_mem.c      | 38 +++++++++++++------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
index 35061ae23ed5..6d4aeb14027b 100644
--- a/tools/testing/selftests/arm64/mte/check_user_mem.c
+++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
@@ -19,12 +19,18 @@
 
 static size_t page_sz;
 
+enum test_type {
+	READ_TEST,
+	LAST_TEST,
+};
+
 static int check_usermem_access_fault(int mem_type, int mode, int mapping,
-                                      int tag_offset, int tag_len)
+                                      int tag_offset, int tag_len,
+                                      enum test_type test_type)
 {
 	int fd, i, err;
 	char val = 'A';
-	size_t len, read_len;
+	ssize_t len, syscall_len;
 	void *ptr, *ptr_next;
 	int fileoff, ptroff, size;
 	int sizes[] = {1, 2, 3, 8, 16, 32, 4096, page_sz};
@@ -46,9 +52,9 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 	}
 	mte_initialize_current_context(mode, (uintptr_t)ptr, len);
 	/* Copy from file into buffer with valid tag */
-	read_len = read(fd, ptr, len);
+	syscall_len = read(fd, ptr, len);
 	mte_wait_after_trig();
-	if (cur_mte_cxt.fault_valid || read_len < len) {
+	if (cur_mte_cxt.fault_valid || syscall_len < len) {
 		err = KSFT_FAIL;
 		goto usermem_acc_err;
 	}
@@ -73,8 +79,16 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 			for (i = 0; i < ARRAY_SIZE(sizes); i++) {
 				size = sizes[i];
 				lseek(fd, 0, 0);
-				/* Copy from file into buffer with invalid tag */
-				read_len = read(fd, ptr + ptroff, size);
+
+				/* perform file operation on buffer with invalid tag */
+				switch (test_type) {
+				case READ_TEST:
+					syscall_len = read(fd, ptr + ptroff, size);
+					break;
+				case LAST_TEST:
+					goto usermem_acc_err;
+				}
+
 				mte_wait_after_trig();
 				/*
 				 * Accessing user memory in kernel with invalid tag should fail in sync
@@ -85,9 +99,9 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 					err = KSFT_FAIL;
 					goto usermem_acc_err;
 				}
-				if (mode == MTE_SYNC_ERR && read_len < len) {
+				if (mode == MTE_SYNC_ERR && syscall_len < len) {
 					/* test passed */
-				} else if (mode == MTE_ASYNC_ERR && read_len == size) {
+				} else if (mode == MTE_ASYNC_ERR && syscall_len == size) {
 					/* test passed */
 				} else {
 					err = KSFT_FAIL;
@@ -121,14 +135,14 @@ int main(int argc, char *argv[])
 	/* Set test plan */
 	ksft_set_plan(4);
 
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE, page_sz, 0),
+	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE, page_sz, 0, READ_TEST),
 		"Check memory access from kernel in sync mode, private mapping and mmap memory\n");
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED, page_sz, 0),
+	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED, page_sz, 0, READ_TEST),
 		"Check memory access from kernel in sync mode, shared mapping and mmap memory\n");
 
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE, page_sz, 0),
+	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE, page_sz, 0, READ_TEST),
 		"Check memory access from kernel in async mode, private mapping and mmap memory\n");
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED, page_sz, 0),
+	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED, page_sz, 0, READ_TEST),
 		"Check memory access from kernel in async mode, shared mapping and mmap memory\n");
 
 	mte_restore_setup();
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/6] kselftest/arm64: mte: user_mem: add test type enum

[Shuah Khan]

On 2/3/22 7:36 AM, Joey Gouly wrote:
> This enum will be used to parameterise the test to perform other syscalls
> than 'read'.
> 

More information here.

> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>   .../selftests/arm64/mte/check_user_mem.c      | 38 +++++++++++++------
>   1 file changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
> index 35061ae23ed5..6d4aeb14027b 100644
> --- a/tools/testing/selftests/arm64/mte/check_user_mem.c
> +++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
> @@ -19,12 +19,18 @@
>   
>   static size_t page_sz;
>   
> +enum test_type {
> +	READ_TEST,
> +	LAST_TEST,
> +};
> +
>   static int check_usermem_access_fault(int mem_type, int mode, int mapping,
> -                                      int tag_offset, int tag_len)
> +                                      int tag_offset, int tag_len,
> +                                      enum test_type test_type)
>   {
>   	int fd, i, err;
>   	char val = 'A';
> -	size_t len, read_len;
> +	ssize_t len, syscall_len;
>   	void *ptr, *ptr_next;
>   	int fileoff, ptroff, size;
>   	int sizes[] = {1, 2, 3, 8, 16, 32, 4096, page_sz};
> @@ -46,9 +52,9 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
>   	}
>   	mte_initialize_current_context(mode, (uintptr_t)ptr, len);
>   	/* Copy from file into buffer with valid tag */
> -	read_len = read(fd, ptr, len);
> +	syscall_len = read(fd, ptr, len);
>   	mte_wait_after_trig();
> -	if (cur_mte_cxt.fault_valid || read_len < len) {
> +	if (cur_mte_cxt.fault_valid || syscall_len < len) {
>   		err = KSFT_FAIL;
>   		goto usermem_acc_err;
>   	}
> @@ -73,8 +79,16 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
>   			for (i = 0; i < ARRAY_SIZE(sizes); i++) {
>   				size = sizes[i];
>   				lseek(fd, 0, 0);
> -				/* Copy from file into buffer with invalid tag */
> -				read_len = read(fd, ptr + ptroff, size);
> +
> +				/* perform file operation on buffer with invalid tag */
> +				switch (test_type) {
> +				case READ_TEST:
> +					syscall_len = read(fd, ptr + ptroff, size);
> +					break;
> +				case LAST_TEST:
> +					goto usermem_acc_err;
> +				}
> +
>   				mte_wait_after_trig();
>   				/*
>   				 * Accessing user memory in kernel with invalid tag should fail in sync
> @@ -85,9 +99,9 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
>   					err = KSFT_FAIL;
>   					goto usermem_acc_err;
>   				}
> -				if (mode == MTE_SYNC_ERR && read_len < len) {
> +				if (mode == MTE_SYNC_ERR && syscall_len < len) {
>   					/* test passed */
> -				} else if (mode == MTE_ASYNC_ERR && read_len == size) {
> +				} else if (mode == MTE_ASYNC_ERR && syscall_len == size) {
>   					/* test passed */
>   				} else {
>   					err = KSFT_FAIL;
> @@ -121,14 +135,14 @@ int main(int argc, char *argv[])
>   	/* Set test plan */
>   	ksft_set_plan(4);
>   
> -	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE, page_sz, 0),
> +	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE, page_sz, 0, READ_TEST),
>   		"Check memory access from kernel in sync mode, private mapping and mmap memory\n");
> -	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED, page_sz, 0),
> +	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED, page_sz, 0, READ_TEST),
>   		"Check memory access from kernel in sync mode, shared mapping and mmap memory\n");
>   
> -	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE, page_sz, 0),
> +	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE, page_sz, 0, READ_TEST),
>   		"Check memory access from kernel in async mode, private mapping and mmap memory\n");
> -	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED, page_sz, 0),
> +	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED, page_sz, 0, READ_TEST),
>   		"Check memory access from kernel in async mode, shared mapping and mmap memory\n");
>   
>   	mte_restore_setup();
> 

The rest looks good

With the above requested changes -

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 5/6] kselftest/arm64: mte: user_mem: add more test types

[Joey Gouly]

Add test support for write, readv, and writev.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
---
 .../selftests/arm64/mte/check_user_mem.c      | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
index 6d4aeb14027b..c128bfa81502 100644
--- a/tools/testing/selftests/arm64/mte/check_user_mem.c
+++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
@@ -11,6 +11,7 @@
 #include <string.h>
 #include <ucontext.h>
 #include <unistd.h>
+#include <sys/uio.h>
 #include <sys/mman.h>
 
 #include "kselftest.h"
@@ -21,6 +22,9 @@ static size_t page_sz;
 
 enum test_type {
 	READ_TEST,
+	WRITE_TEST,
+	READV_TEST,
+	WRITEV_TEST,
 	LAST_TEST,
 };
 
@@ -85,6 +89,23 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 				case READ_TEST:
 					syscall_len = read(fd, ptr + ptroff, size);
 					break;
+				case WRITE_TEST:
+					syscall_len = write(fd, ptr + ptroff, size);
+					break;
+				case READV_TEST: {
+					struct iovec iov[1];
+					iov[0].iov_base = ptr + ptroff;
+					iov[0].iov_len = size;
+					syscall_len = readv(fd, iov, 1);
+					break;
+				}
+				case WRITEV_TEST: {
+					struct iovec iov[1];
+					iov[0].iov_base = ptr + ptroff;
+					iov[0].iov_len = size;
+					syscall_len = writev(fd, iov, 1);
+					break;
+				}
 				case LAST_TEST:
 					goto usermem_acc_err;
 				}
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 5/6] kselftest/arm64: mte: user_mem: add more test types

[Shuah Khan]

On 2/3/22 7:36 AM, Joey Gouly wrote:
> Add test support for write, readv, and writev.

A bit more info. what this adds.

> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> ---
>   .../selftests/arm64/mte/check_user_mem.c      | 21 +++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 


The rest looks good

With the above requested changes -

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 6/6] kselftest/arm64: mte: user_mem: test more combinations of values

[Joey Gouly]

Expand the tests to cover more syscalls, tag offsets and tag lengths.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
---
 .../selftests/arm64/mte/check_user_mem.c      | 94 ++++++++++++++++---
 1 file changed, 83 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/check_user_mem.c b/tools/testing/selftests/arm64/mte/check_user_mem.c
index c128bfa81502..1aef855bf81a 100644
--- a/tools/testing/selftests/arm64/mte/check_user_mem.c
+++ b/tools/testing/selftests/arm64/mte/check_user_mem.c
@@ -3,6 +3,7 @@
 
 #define _GNU_SOURCE
 
+#include <assert.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <signal.h>
@@ -20,6 +21,8 @@
 
 static size_t page_sz;
 
+#define TEST_NAME_MAX 100
+
 enum test_type {
 	READ_TEST,
 	WRITE_TEST,
@@ -137,9 +140,67 @@ static int check_usermem_access_fault(int mem_type, int mode, int mapping,
 	return err;
 }
 
+void format_test_name(char* name, int name_len, int type, int sync, int map, int len, int offset) {
+	const char* test_type;
+	const char* mte_type;
+	const char* map_type;
+
+	switch (type) {
+	case READ_TEST:
+		test_type = "read";
+		break;
+	case WRITE_TEST:
+		test_type = "write";
+		break;
+	case READV_TEST:
+		test_type = "readv";
+		break;
+	case WRITEV_TEST:
+		test_type = "writev";
+		break;
+	default:
+		assert(0);
+		break;
+	}
+
+	switch (sync) {
+	case MTE_SYNC_ERR:
+		mte_type = "MTE_SYNC_ERR";
+		break;
+	case MTE_ASYNC_ERR:
+		mte_type = "MTE_ASYNC_ERR";
+		break;
+	default:
+		assert(0);
+		break;
+	}
+
+	switch (map) {
+	case MAP_SHARED:
+		map_type = "MAP_SHARED";
+		break;
+	case MAP_PRIVATE:
+		map_type = "MAP_PRIVATE";
+		break;
+	default:
+		assert(0);
+		break;
+	}
+
+	snprintf(name, name_len,
+	         "test type: %s, %s, %s, tag len: %d, tag offset: %d\n",
+	         test_type, mte_type, map_type, len, offset);
+}
+
 int main(int argc, char *argv[])
 {
 	int err;
+	int t, s, m, l, o;
+	int mte_sync[] = {MTE_SYNC_ERR, MTE_ASYNC_ERR};
+	int maps[] = {MAP_SHARED, MAP_PRIVATE};
+	int tag_lens[] = {0, MT_GRANULE_SIZE};
+	int tag_offsets[] = {page_sz, MT_GRANULE_SIZE};
+	char test_name[TEST_NAME_MAX];
 
 	page_sz = getpagesize();
 	if (!page_sz) {
@@ -154,17 +215,28 @@ int main(int argc, char *argv[])
 	mte_register_signal(SIGSEGV, mte_default_handler);
 
 	/* Set test plan */
-	ksft_set_plan(4);
-
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_PRIVATE, page_sz, 0, READ_TEST),
-		"Check memory access from kernel in sync mode, private mapping and mmap memory\n");
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_SYNC_ERR, MAP_SHARED, page_sz, 0, READ_TEST),
-		"Check memory access from kernel in sync mode, shared mapping and mmap memory\n");
-
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_PRIVATE, page_sz, 0, READ_TEST),
-		"Check memory access from kernel in async mode, private mapping and mmap memory\n");
-	evaluate_test(check_usermem_access_fault(USE_MMAP, MTE_ASYNC_ERR, MAP_SHARED, page_sz, 0, READ_TEST),
-		"Check memory access from kernel in async mode, shared mapping and mmap memory\n");
+	ksft_set_plan(64);
+
+	for (t = 0; t < LAST_TEST; t++) {
+		for (s = 0; s < ARRAY_SIZE(mte_sync); s++) {
+			for (m = 0; m < ARRAY_SIZE(maps); m++) {
+				for (l = 0; l < ARRAY_SIZE(tag_lens); l++) {
+					for (o = 0; o < ARRAY_SIZE(tag_offsets); o++) {
+						int sync = mte_sync[s];
+						int map = maps[m];
+						int offset = tag_offsets[o];
+						int tag_len = tag_lens[l];
+						int res = check_usermem_access_fault(USE_MMAP, sync,
+						                                     map, offset,
+						                                     tag_len, t);
+						format_test_name(test_name, TEST_NAME_MAX,
+						                 t, sync, map, tag_len, offset);
+						evaluate_test(res, test_name);
+					}
+				}
+			}
+		}
+	}
 
 	mte_restore_setup();
 	ksft_print_cnts();
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 6/6] kselftest/arm64: mte: user_mem: test more combinations of values

[Shuah Khan]

On 2/3/22 7:36 AM, Joey Gouly wrote:
> Expand the tests to cover more syscalls, tag offsets and tag lengths.
> 

You guessed it. I am going to ask for which syscalls are covered with
this patch :)

> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> ---


The rest looks good

With the above requested changes -

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/6] add more tests to MTE kselftests

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 440 bytes --]

On Thu, Feb 03, 2022 at 02:36:14PM +0000, Joey Gouly wrote:
> This is a series which refactors and then adds some extra tests for MTE
> in the kselftest framework.
> 
> The issue that these tests are for was was fixed by Robin in
> 295cf156231c ("arm64: Avoid premature usercopy failure") and based on a
> simplified example by Catalin [1].

Reviewed-by: Mark Brown <broonie@kernel.org>
Tested-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 0/6] add more tests to MTE kselftests

[Shuah Khan]

On 2/3/22 7:36 AM, Joey Gouly wrote:
> This is a series which refactors and then adds some extra tests for MTE
> in the kselftest framework.
> 
> The issue that these tests are for was was fixed by Robin in
> 295cf156231c ("arm64: Avoid premature usercopy failure") and based on a
> simplified example by Catalin [1].
> 
> They test some combinations of pointer offsets, sizes and syscalls to
> exercise different paths in the kernel.
> 
> Changes since v1 [2]:
>    - add a page_sz to the tests, for non-4K kernels
>    - use loops to generate more tests and avoid long lines
> 
> Thanks,
> Joey
> 
> [1] https://lore.kernel.org/all/20210624150911.GA25097@arm.com/
> [2] https://lore.kernel.org/linux-arm-kernel/20220125150920.23955-1-joey.gouly@arm.com/
> 
> Joey Gouly (6):
>    kselftest/arm64: mte: user_mem: introduce tag_offset and tag_len
>    kselftest/arm64: mte: user_mem: rework error handling
>    kselftest/arm64: mte: user_mem: check different offsets and sizes
>    kselftest/arm64: mte: user_mem: add test type enum
>    kselftest/arm64: mte: user_mem: add more test types
>    kselftest/arm64: mte: user_mem: test more combinations of values
> 
>   .../selftests/arm64/mte/check_user_mem.c      | 196 +++++++++++++++---
>   1 file changed, 162 insertions(+), 34 deletions(-)
> 

Just noticed that these patches aren't cc'ed to linux-kselftest list. Please
do so when you sen v2.

thanks,
-- Shuah

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel