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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox