From: Kris Van Hees <kris.van.hees@oracle.com>
To: Sam James <sam@gentoo.org>
Cc: Kris Van Hees via DTrace-devel <dtrace-devel@oss.oracle.com>,
dtrace@lists.linux.dev, Kris Van Hees <kris.van.hees@oracle.com>
Subject: Re: [DTrace-devel] [PATCH] bpf: force generating code that all verifiers accept
Date: Thu, 14 Aug 2025 16:15:36 -0400 [thread overview]
Message-ID: <aJ5D6HVguDBu8Ww3@oracle.com> (raw)
In-Reply-To: <87h5y9k7gm.fsf@gentoo.org>
On Thu, Aug 14, 2025 at 08:51:21PM +0100, Sam James wrote:
> 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.
Thanks for the feedback - good idea.
> > 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
prev parent reply other threads:[~2025-08-14 20:15 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-13 21:18 [PATCH] bpf: force generating code that all verifiers accept Kris Van Hees
2025-08-14 16:32 ` Eugene Loh
2025-08-14 19:51 ` [DTrace-devel] " Sam James
2025-08-14 20:15 ` Kris Van Hees [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aJ5D6HVguDBu8Ww3@oracle.com \
--to=kris.van.hees@oracle.com \
--cc=dtrace-devel@oss.oracle.com \
--cc=dtrace@lists.linux.dev \
--cc=sam@gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.