[DTrace-devel] [PATCH] test: Use copyinstr() and copyin() when using data in user buffers

Kris Van Hees kris.van.hees at oracle.com
Sun Feb 26 04:05:25 UTC 2023 

On Sat, Feb 25, 2023 at 03:19:06PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d | 4 ++--
>  test/unittest/funcs/strtok/tst.strtok_nonDPTR.d   | 8 ++++----
>  test/unittest/funcs/substr/tst.substr_nonDPTR.d   | 6 +++---
>  test/unittest/funcs/tst.basename_nonDPTR.d        | 4 ++--
>  test/unittest/funcs/tst.dirname_nonDPTR.d         | 4 ++--
>  test/unittest/funcs/tst.index_nonDPTR.d           | 4 ++--
>  test/unittest/funcs/tst.inet_ntoa_nonDPTR.d       | 4 ++--
>  test/unittest/funcs/tst.rindex_nonDPTR.d          | 4 ++--
>  test/unittest/funcs/tst.strchr_nonDPTR.d          | 6 +++---
>  9 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d b/test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d
> index 433ceeb9..5734475c 100644
> --- a/test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d
> +++ b/test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -17,7 +17,7 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("|%s|\n", strjoin((void *)arg1, (void *)arg1));
> +	printf("|%s|\n", strjoin(copyinstr(arg1), copyinstr(arg1)));

The problem with this change is that you go from pointers that you cannot
access directly on some platforms to using copyinstr() which copies the
strings into tstrings before calling strjoin() which (I think) invalidates
the testing.  Sure, tstrings are not marked DPTR but they are in DTrace
managed memory anyway.  In other words, if the strjoin() were to invalidly
use indirect loads on the pointers it would actually work.

Instead, I think we should use something like:

	printf("|%s|\n", strjoin(`linux_banner, `linux_banner));

And I would expect the same applies for the other tests...

And those that use copyin() will end up with the data in ALLOCA memory which
again allows indirect loads...  So that is an issue also.

>  	exit(0);
>  }
>  
> diff --git a/test/unittest/funcs/strtok/tst.strtok_nonDPTR.d b/test/unittest/funcs/strtok/tst.strtok_nonDPTR.d
> index 54da5650..9c440686 100644
> --- a/test/unittest/funcs/strtok/tst.strtok_nonDPTR.d
> +++ b/test/unittest/funcs/strtok/tst.strtok_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -17,10 +17,10 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("|%s|\n", strtok((void *)arg1, "defghidEFGHI"));
> +	printf("|%s|\n", strtok(copyinstr(arg1), "defghidEFGHI"));
>  	printf("|%s|\n", strtok(NULL, "fF"));
> -	printf("|%s|\n", strtok("nmlkjihgfFOOBARedcba", (void *)arg1));
> -	printf("|%s|\n", strtok(NULL, (void *)arg1));
> +	printf("|%s|\n", strtok("nmlkjihgfFOOBARedcba", copyinstr(arg1)));
> +	printf("|%s|\n", strtok(NULL, copyinstr(arg1)));
>  	exit(0);
>  }
>  
> diff --git a/test/unittest/funcs/substr/tst.substr_nonDPTR.d b/test/unittest/funcs/substr/tst.substr_nonDPTR.d
> index 726f20a8..014a1798 100644
> --- a/test/unittest/funcs/substr/tst.substr_nonDPTR.d
> +++ b/test/unittest/funcs/substr/tst.substr_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -17,8 +17,8 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("|%s|\n", substr((void *)arg1, 1));
> -	printf("|%s|\n", substr((void *)arg1, 1, 4));
> +	printf("|%s|\n", substr(copyinstr(arg1), 1));
> +	printf("|%s|\n", substr(copyinstr(arg1), 1, 4));
>  	exit(0);
>  }
>  
> diff --git a/test/unittest/funcs/tst.basename_nonDPTR.d b/test/unittest/funcs/tst.basename_nonDPTR.d
> index 12f6436c..0538ab9f 100644
> --- a/test/unittest/funcs/tst.basename_nonDPTR.d
> +++ b/test/unittest/funcs/tst.basename_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -17,7 +17,7 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("|%s|\n", basename((void *)arg1));
> +	printf("|%s|\n", basename(copyinstr(arg1)));
>  	exit(0);
>  }
>  
> diff --git a/test/unittest/funcs/tst.dirname_nonDPTR.d b/test/unittest/funcs/tst.dirname_nonDPTR.d
> index dab80655..32026280 100644
> --- a/test/unittest/funcs/tst.dirname_nonDPTR.d
> +++ b/test/unittest/funcs/tst.dirname_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -17,7 +17,7 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("|%s|\n", dirname((void *)arg1));
> +	printf("|%s|\n", dirname(copyinstr(arg1)));
>  	exit(0);
>  }
>  
> diff --git a/test/unittest/funcs/tst.index_nonDPTR.d b/test/unittest/funcs/tst.index_nonDPTR.d
> index 44a9d6df..9d24c854 100644
> --- a/test/unittest/funcs/tst.index_nonDPTR.d
> +++ b/test/unittest/funcs/tst.index_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -17,7 +17,7 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("index is %d, should be 0\n", index((void *)arg1, (void *)arg1));
> +	printf("index is %d, should be 0\n", index(copyinstr(arg1), copyinstr(arg1)));
>  	exit(0);
>  }
>  
> diff --git a/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d b/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
> index 3622787d..4389d00d 100644
> --- a/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
> +++ b/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -16,7 +16,7 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("%s\n", inet_ntoa((void *)arg1));
> +	printf("%s\n", inet_ntoa(copyin(arg1, 4)));
>  	exit(0);
>  }
>  
> diff --git a/test/unittest/funcs/tst.rindex_nonDPTR.d b/test/unittest/funcs/tst.rindex_nonDPTR.d
> index 0b193eb0..9bc5ff06 100644
> --- a/test/unittest/funcs/tst.rindex_nonDPTR.d
> +++ b/test/unittest/funcs/tst.rindex_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -17,7 +17,7 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("rindex is %d, should be 0\n", rindex((void *)arg1, (void *)arg1));
> +	printf("rindex is %d, should be 0\n", rindex(copyinstr(arg1), copyinstr(arg1)));
>  	exit(0);
>  }
>  
> diff --git a/test/unittest/funcs/tst.strchr_nonDPTR.d b/test/unittest/funcs/tst.strchr_nonDPTR.d
> index 95cb08c2..5b5595f6 100644
> --- a/test/unittest/funcs/tst.strchr_nonDPTR.d
> +++ b/test/unittest/funcs/tst.strchr_nonDPTR.d
> @@ -1,6 +1,6 @@
>  /*
>   * Oracle Linux DTrace.
> - * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> @@ -17,8 +17,8 @@ BEGIN
>  syscall::write:entry
>  /ppid == $pid/
>  {
> -	printf("|%s|\n", strchr((void *)arg1, 'b'));
> -	printf("|%s|\n", strrchr((void *)arg1, 'b'));
> +	printf("|%s|\n", strchr(copyinstr(arg1), 'b'));
> +	printf("|%s|\n", strrchr(copyinstr(arg1), 'b'));
>  	exit(0);
>  }
>  
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

More information about the DTrace-devel mailing list