[DTrace-devel] [PATCH] Support inet_ntoa() for addresses that are not DTrace pointers

[Kris Van Hees]

On Thu, Oct 13, 2022 at 02:58:36PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> If the input argument to inet_ntoa() is not a DTrace pointer, the
> BPF verifier cannot confirm it is safe.  So first copy the input
> data to the BPF stack using the probe_read() helper function.

I think I'd rather see this done at the CG level, i.e. determine whether the
node is DT_NF_DPTR and if so, copy and pass that copy to the function?  That
way the verifier *will* validate mempry accesses when possible.

> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  bpf/inet_ntoa.S                             | 30 +++++++++++++--------
>  test/unittest/funcs/tst.inet_ntoa_nonDPTR.d | 23 ++++++++++++++++
>  test/unittest/funcs/tst.inet_ntoa_nonDPTR.r |  2 ++
>  3 files changed, 44 insertions(+), 11 deletions(-)
>  create mode 100644 test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
>  create mode 100644 test/unittest/funcs/tst.inet_ntoa_nonDPTR.r
> 
> diff --git a/bpf/inet_ntoa.S b/bpf/inet_ntoa.S
> index ff36e226..a6676ff2 100644
> --- a/bpf/inet_ntoa.S
> +++ b/bpf/inet_ntoa.S
> @@ -3,6 +3,8 @@
>   * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
>   */
>  
> +#define BPF_FUNC_probe_read	4
> +
>  	.text
>  
>  /*
> @@ -115,25 +117,27 @@ dt_inet_ntoa_write_uint8:
>   * void dt_inet_ntoa(uint8_t *src, char *dst) {
>   *     uint64_t off, inp, len;
>   *
> - *     off = 0
> + *     bpf_probe_read(fp + -4, 4, src);
> + *
> + *     off = 0;
>   *     len = STRSZ;
>   *
> - *     inp = src[0];
> + *     inp = *(fp + -4);
>   *     off = dt_inet_ntoa_write_uint8(inp, dst, off);
>   *     if (off >= STRSZ) goto done:
>   *     dst[off++] = ':';
>   *
> - *     inp = src[1];
> + *     inp = *(fp + -3);
>   *     off = dt_inet_ntoa_write_uint8(inp, dst, off);
>   *     if (off >= STRSZ) goto done:
>   *     dst[off++] = ':';
>   *
> - *     inp = src[2];
> + *     inp = *(fp + -2);
>   *     off = dt_inet_ntoa_write_uint8(inp, dst, off);
>   *     if (off >= STRSZ) goto done:
>   *     dst[off++] = ':';
>   *
> - *     inp = src[3];
> + *     inp = *(fp + -1);
>   *     off = dt_inet_ntoa_write_uint8(inp, dst, off);
>   *
>   *     done:
> @@ -148,17 +152,21 @@ dt_inet_ntoa:
>  
>  /* off cycles between %r3 (input arg to subroutine) and %r0 (its return) */
>  #define INP %r1
> -#define SRC %r6
>  #define DST %r7
>  #define LEN %r8
>  
> -	mov	SRC, %r1
>  	mov	DST, %r2
>  
> +	mov	%r3, %r1
> +	mov	%r2, 4
> +	mov	%r1, %fp
> +	add	%r1, -4
> +	call	BPF_FUNC_probe_read
> +
>  	mov	%r3, 0
>  	lddw	LEN, STRSZ
>  
> -	ldxb	INP, [SRC+0]
> +	ldxb	INP, [%fp+-4]
>  	mov	%r2, DST
>  	call	dt_inet_ntoa_write_uint8
>  	jge	%r0, LEN, .Ldone
> @@ -167,7 +175,7 @@ dt_inet_ntoa:
>  	add	%r0, DST
>  	stb	[%r0+0], '.'
>  
> -	ldxb	INP, [SRC+1]
> +	ldxb	INP, [%fp+-3]
>  	mov	%r2, DST
>  	call	dt_inet_ntoa_write_uint8
>  	jge	%r0, LEN, .Ldone
> @@ -176,7 +184,7 @@ dt_inet_ntoa:
>  	add	%r0, DST
>  	stb	[%r0+0], '.'
>  
> -	ldxb	INP, [SRC+2]
> +	ldxb	INP, [%fp+-2]
>  	mov	%r2, DST
>  	call	dt_inet_ntoa_write_uint8
>  	jge	%r0, LEN, .Ldone
> @@ -185,7 +193,7 @@ dt_inet_ntoa:
>  	add	%r0, DST
>  	stb	[%r0+0], '.'
>  
> -	ldxb	INP, [SRC+3]
> +	ldxb	INP, [%fp+-1]
>  	mov	%r2, DST
>  	call	dt_inet_ntoa_write_uint8
>  
> diff --git a/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d b/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
> new file mode 100644
> index 00000000..9b51dad4
> --- /dev/null
> +++ b/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
> @@ -0,0 +1,23 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2022, 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.
> + */
> +
> +#pragma D option quiet
> +#pragma D option destructive
> +
> +BEGIN
> +{
> +        system("printf '\xc0\xa8\x01\x17' > /dev/null 2>&1");
> +}
> +
> +syscall::write:entry
> +/ppid == $pid/
> +{
> +        printf("%s\n", inet_ntoa((void *)arg1));
> +        exit(0);
> +}
> +
> +ERROR { exit(1); }
> diff --git a/test/unittest/funcs/tst.inet_ntoa_nonDPTR.r b/test/unittest/funcs/tst.inet_ntoa_nonDPTR.r
> new file mode 100644
> index 00000000..7257e488
> --- /dev/null
> +++ b/test/unittest/funcs/tst.inet_ntoa_nonDPTR.r
> @@ -0,0 +1,2 @@
> +192.168.1.23
> +
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel