[DTrace-devel] [PATCH] Use highest bit to decide if PC is user space or kernel

[Kris Van Hees]

On Mon, Feb 27, 2023 at 07:23:29PM -0500, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Currently, we decide if a PC is user space or kernel based on whether
> the BPF helper function probe_read_kernel() can read the address.
> On systems where this BPF helper function does not exist, there is a
> back-up mechanism that simply checks the highest bit of the PC.
> 
> Apparently, on some systems, the helper function can read addresses
> even in user space.  That is, the mechanism that uses the helper function
> is not reliable.
> 
> So, just use the "back-up mechanism" all the time.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>

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

... even though it is (sadly) a very ugly patch.  But necessary and I do agree
that keeping the existing code around makes a lot of sense here because there
ought to be a better way to handle this.

> ---
>  libdtrace/dt_cg.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 4791536b..ac99d3dd 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -414,7 +414,6 @@ dt_cg_tramp_copy_args_from_regs(dt_pcb_t *pcb, int called)
>  void
>  dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb)
>  {
> -	dtrace_hdl_t	*dtp = pcb->pcb_hdl;
>  	dt_regset_t	*drp = pcb->pcb_regs;
>  	dt_irlist_t	*dlp = &pcb->pcb_ir;
>  	uint_t		Luser = dt_irlist_label(dlp);
> @@ -429,7 +428,18 @@ dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb)
>          emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(1), BPF_REG_3));
>  
>  	/* check if the PC is kernel or user space */
> -	if (dtp->dt_bpfhelper[BPF_FUNC_probe_read_kernel] == BPF_FUNC_probe_read_kernel) {
> +#if 0
> +	if (pcb->pcb_hdl->dt_bpfhelper[BPF_FUNC_probe_read_kernel]
> +	    == BPF_FUNC_probe_read_kernel) {
> +#else
> +	/*
> +	 * We do not seem to be guaranteed for some kernels that
> +	 * probe_read_kernel(), even if it exists, will signal an error
> +	 * when reading user space addresses.  So, do not use the
> +	 * probe_read_kernel() mechanism for now.
> +	 */
> +	if (0) {
> +#endif
>  		/*
>  		 * Use probe_read_kernel() if it is really is probe_read_kernel().
>  		 * On older kernels, it does not exist and is aliased to something else.
> @@ -452,7 +462,6 @@ dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb)
>  	} else {
>  		/*
>  		 * If no real probe_read_kernel() exists, just test the highest bit.
> -		 * This is not as robust, but probably works just fine for us.
>  		 */
>  
>  		/* if the highest bit is 0, assume it was user space */
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel