[DTrace-devel] [PATCH v2 3/4] uregs: Fix access to thread members on x86

Kris Van Hees kris.van.hees at oracle.com
Tue May 23 05:13:41 UTC 2023


On Fri, May 19, 2023 at 01:49:46PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> The "fix" is simply to account for thread members that are less than
> 8 bytes wide.
> 
> Most of this patch is for refactoring (using functions to determine
> offsets with CTF types or to load a scalar safely from memory that
> the BPF verifier cannot vet) and adding a test.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

> ---
>  libdtrace/dt_cg.c                             | 62 +++++++------------
>  test/unittest/arrays/tst.uregsarray-check2.d  | 32 ++++++++++
>  test/unittest/arrays/tst.uregsarray-check2.r  |  5 ++
>  .../unittest/arrays/tst.uregsarray-check2.r.p |  5 ++
>  test/unittest/arrays/tst.uregsarray-check2.x  |  4 ++
>  5 files changed, 70 insertions(+), 38 deletions(-)
>  create mode 100644 test/unittest/arrays/tst.uregsarray-check2.d
>  create mode 100644 test/unittest/arrays/tst.uregsarray-check2.r
>  create mode 100755 test/unittest/arrays/tst.uregsarray-check2.r.p
>  create mode 100755 test/unittest/arrays/tst.uregsarray-check2.x
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index e431fb36..74eb4a31 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -4132,57 +4132,43 @@ dt_cg_uregs(unsigned int idx, dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp
>  	if (idx >= sizeof(dt_pt_regs) / sizeof(uint64_t)) {
>  
>  #if defined(__amd64)
> +		/* Confirm that we can hardwire for 21 pt_regs[]. */
> +		assert(sizeof(dt_pt_regs) == 21 * sizeof(uint64_t));
> +
>  		/*
>  		 * Even if out-of-bounds, on x86 there are still a few
>  		 * indices that are used to access task->thread. members.
>  		 */
>  		if (idx <= 25) {
> -			ctf_file_t *cfp = dtp->dt_shared_ctf;
> -			ctf_id_t type;
> -			ctf_membinfo_t ctm;
> -			int offset, rc;
> -
> -			/* look up task->thread offset */
> -			if (!cfp)
> -				longjmp(yypcb->pcb_jmpbuf, EDT_NOCTF);
> -
> -			type = ctf_lookup_by_name(cfp, "struct task_struct");
> -			if (type == CTF_ERR)
> -				longjmp(yypcb->pcb_jmpbuf, EDT_NOCTF);
> -			if (ctf_member_info(cfp, type, "thread", &ctm) == CTF_ERR)
> -				longjmp(yypcb->pcb_jmpbuf, EDT_NOCTF);
> -			offset = ctm.ctm_offset / NBBY;
> -
> -			/* add the thread->member offset */
> -			type = ctf_lookup_by_name(cfp, "struct thread_struct");
> -			if (type == CTF_ERR)
> -				longjmp(yypcb->pcb_jmpbuf, EDT_NOCTF);
> -			switch (idx) {
> -			case 21: rc = ctf_member_info(cfp, type, "ds", &ctm); break;
> -			case 22: rc = ctf_member_info(cfp, type, "es", &ctm); break;
> -			case 23: rc = ctf_member_info(cfp, type, "fsbase", &ctm); break;
> -			case 24: rc = ctf_member_info(cfp, type, "gsbase", &ctm); break;
> -			case 25: rc = ctf_member_info(cfp, type, "trap_nr", &ctm); break;
> -			}
> -			if (rc == -1)
> -				longjmp(yypcb->pcb_jmpbuf, EDT_NOCTF);
> -			offset += ctm.ctm_offset / NBBY;
> +			int offset;
> +			ssize_t size;
> +			char *memnames[] = { "ds", "es", "fsbase", "gsbase", "trap_nr" };
> +
> +			/* Look up task->thread offset. */
> +			offset = dt_cg_ctf_offsetof("struct task_struct", "thread", NULL);
>  
> -			/* copy task->thread.member onto the stack */
> +			/* Add the thread->member offset. */
> +			offset += dt_cg_ctf_offsetof("struct thread_struct",
> +						     memnames[idx - 21], &size);
> +			if (size < 1 || size > 8 || (size & (size - 1)) != 0)
> +				xyerror(D_UNKNOWN, "internal error -- cg cannot load "
> +				    "size %ld when passed by value\n", (long)size);
> +
> +			/* Get task. */
>  			if (dt_regset_xalloc_args(drp) == -1)
>  				longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  			dt_regset_xalloc(drp, BPF_REG_0);
>  			emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_task));
> -			emit(dlp, BPF_MOV_REG(BPF_REG_3, BPF_REG_0));
> -			emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, offset));
> -			emit(dlp, BPF_MOV_IMM(BPF_REG_2, sizeof(uint64_t)));
> -			emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_SP));
> -			emit(dlp, BPF_CALL_HELPER(dtp->dt_bpfhelper[BPF_FUNC_probe_read_kernel]));
>  			dt_regset_free_args(drp);
> -			emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_FP, DT_STK_SP));
> -			emit(dlp, BPF_LOAD(BPF_DW, dnp->dn_reg, BPF_REG_0, 0));
> +			emit(dlp, BPF_MOV_REG(dnp->dn_reg, BPF_REG_0));
>  			dt_regset_free(drp, BPF_REG_0);
>  
> +			/* Add the offset to the task pointer. */
> +			emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_reg, offset));
> +
> +			/* Dereference it safely (the BPF verifier has no idea what it is). */
> +			dt_cg_load_scalar(dnp, ldstw[size], size, dlp, drp);
> +
>  			return;
>  		}
>  #endif
> diff --git a/test/unittest/arrays/tst.uregsarray-check2.d b/test/unittest/arrays/tst.uregsarray-check2.d
> new file mode 100644
> index 00000000..de65bbfd
> --- /dev/null
> +++ b/test/unittest/arrays/tst.uregsarray-check2.d
> @@ -0,0 +1,32 @@
> +/*
> + * Oracle Linux DTrace.
> + * 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.
> + */
> +
> +/*
> + * ASSERTION:  Check the constants used to access uregs[].
> + *	       On x86, certain high indices are used to access not
> + *	       pt_regs[] but curthread->thread.xxx.  So check them.
> + *
> + * SECTION: User Process Tracing/uregs Array
> + */
> +
> +#pragma D option quiet
> +
> +BEGIN
> +{
> +	printf("%x %x\n", uregs[R_DS    ], curthread->thread.ds);
> +	printf("%x %x\n", uregs[R_ES    ], curthread->thread.ds);
> +	printf("%x %x\n", uregs[R_FS    ], curthread->thread.fsbase);
> +	printf("%x %x\n", uregs[R_GS    ], curthread->thread.gsbase);
> +	printf("%x %x\n", uregs[R_TRAPNO], curthread->thread.trap_nr);
> +
> +        exit(0);
> +}
> +
> +ERROR
> +{
> +	exit(1);
> +}
> diff --git a/test/unittest/arrays/tst.uregsarray-check2.r b/test/unittest/arrays/tst.uregsarray-check2.r
> new file mode 100644
> index 00000000..05c0b188
> --- /dev/null
> +++ b/test/unittest/arrays/tst.uregsarray-check2.r
> @@ -0,0 +1,5 @@
> +match
> +match
> +match
> +match
> +match
> diff --git a/test/unittest/arrays/tst.uregsarray-check2.r.p b/test/unittest/arrays/tst.uregsarray-check2.r.p
> new file mode 100755
> index 00000000..2f53ea95
> --- /dev/null
> +++ b/test/unittest/arrays/tst.uregsarray-check2.r.p
> @@ -0,0 +1,5 @@
> +#!/usr/bin/gawk -f
> +
> +NF == 0 { next }
> +$1 == $2 { print "match" }
> +$1 != $2 { print "ERROR" }
> diff --git a/test/unittest/arrays/tst.uregsarray-check2.x b/test/unittest/arrays/tst.uregsarray-check2.x
> new file mode 100755
> index 00000000..69e36e70
> --- /dev/null
> +++ b/test/unittest/arrays/tst.uregsarray-check2.x
> @@ -0,0 +1,4 @@
> +#!/bin/sh
> +
> +[ `uname -m` = "x86_64" ] && exit 0
> +exit 2
> -- 
> 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