Re: [DTrace-devel] [PATCH] bpf: force generating code that all verifiers accept

[Sam James <sam@gentoo.org>]

Kris Van Hees via DTrace-devel <dtrace-devel@oss.oracle.com> writes:

> The compiler could optimize val = *valp in a way where the verifier on
> older kernels would complain.  We use inline assembler to force the
> not optimize this expression and instead to always read the value as a
> scalar.

I'd mention a known-bad kernel version and some testcase which
definitely hit it, so we know when we can remove it in future.

>
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>  bpf/get_dvar.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/bpf/get_dvar.c b/bpf/get_dvar.c
> index 073cca57c..aa14eca58 100644
> --- a/bpf/get_dvar.c
> +++ b/bpf/get_dvar.c
> @@ -150,7 +150,24 @@ noinline void *dt_get_assoc(uint32_t id, const char *tuple, uint64_t store,
>  		if (valp == 0)
>  			return dt_no_dvar();
>  		*valp = (uint64_t)valp;
> -		val = *valp;
> +		/*
> +		 * We used to do:
> +		 *	val = *valp;
> +		 * but the compiler could use knowledge that *valp is valp from
> +		 * the assignment above, and use that same value (whith is a
> +		 * map_value address).  Older kernels do not allow a map_value
> +		 * address to be used as map key, and a verifier failure would
> +		 * be triggered by this code optimization.
> +		 *
> +		 * We use inline assembler to force reading the value from the
> +		 * map value rather than allowing the compiler to optimize this
> +		 * code.  This works for all kernels.
> +		 */
> +		asm ("ldxdw %0, %1" \
> +			: "=r" (val) \
> +			: "m" (*valp) \
> +			: /* no clobber */
> +		);
>  	} else {
>  		/*
>  		 * Record the value (used as key into the dvars map), and if we