[DTrace-devel] [PATCH 2/2] rawfbt: prvname is not properly set
Eugene Loh
eugene.loh at oracle.com
Fri Jan 16 05:18:51 UTC 2026
On 1/15/26 17:18, Kris Van Hees wrote:
> On Tue, Jan 13, 2026 at 04:42:05PM -0500, eugene.loh at oracle.com wrote:
>> From: Eugene Loh <eugene.loh at oracle.com>
>>
>> The char array prvname[] is set for each provider. It is used in the
>> file that implements the provider. It might also be passed to
>> dt_sdt_populate() via a function argument.
>>
>> However, dt_provider_tp.h also defines the macro GROUP_DATA in terms of
>> prvname. In turn, GROUP_DATA is used not only in dt_prov_dtrace.c but
>> then again in dt_prov_fbt.c to define FBT_GROUP_DATA.
>>
>> In commit 0b7c5a632 ("fbt, rawfbt: consolidate code to avoid duplication"),
>> the fbt and rawfbt providers are combined into a single file. Thus, two
>> uses of prvname collide. The in-file collisions get resolved, but the
>> cascade of macro definitions does not: FBT_GROUP_DATA ends up using
>> "fbt" for both fbt and rawfbt providers. As a result, it is possible
>> for rawfbt probes not to be seen.
>>
>> Notice that GROUP_DATA is always paired with prp->desc->prb. Therefore,
>> simply replace:
>> -#define GROUP_DATA getpid(), prvname
>> +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb
>> This makes the code more compact and relieves the macro definitions from
>> needing prvname. It also makes the FBT_GROUP_DATA macro unnecessary.
>>
>> The format (FMT) macro is similarly renamed and redefined to include the
>> probe info.
>>
>> Add a test to check that a rawfbt probe can be found behind an fbt probe
>> when the probe description has a wildcard provider.
>>
>> Orabug: 38842114
>> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
>> ---
>> libdtrace/dt_prov_dtrace.c | 14 ++++++-------
>> libdtrace/dt_prov_fbt.c | 13 ++++++------
>> libdtrace/dt_prov_rawtp.c | 4 ++--
>> libdtrace/dt_prov_sdt.c | 4 ++--
>> libdtrace/dt_provider_tp.h | 12 +++++------
>> .../providers/rawfbt/tst.wildcard-provider.d | 20 +++++++++++++++++++
>> .../providers/rawfbt/tst.wildcard-provider.r | 2 ++
>> 7 files changed, 45 insertions(+), 24 deletions(-)
>> create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.d
>> create mode 100644 test/unittest/providers/rawfbt/tst.wildcard-provider.r
>>
>> diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
>> index 34b5d8e24..102afd84e 100644
>> --- a/libdtrace/dt_prov_dtrace.c
>> +++ b/libdtrace/dt_prov_dtrace.c
>> @@ -243,8 +243,8 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
>> /* add a uprobe */
>> fd = open(UPROBE_EVENTS, O_WRONLY | O_APPEND);
>> if (fd != -1) {
>> - rc = dprintf(fd, "p:" GROUP_FMT "/%s %s\n",
>> - GROUP_DATA, prp->desc->prb, spec);
>> + rc = dprintf(fd, "p:" PROBE_FMT " %s\n",
>> + PROBE_DATA, spec);
>> close(fd);
>> }
>> free(spec);
>> @@ -252,14 +252,14 @@ static int attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
>> return -ENOENT;
>>
>> /* open format file */
>> - len = snprintf(NULL, 0, "%s" GROUP_FMT "/%s/format",
>> - EVENTSFS, GROUP_DATA, prp->desc->prb) + 1;
>> + len = snprintf(NULL, 0, "%s" PROBE_FMT "/format",
>> + EVENTSFS, PROBE_DATA) + 1;
>> fn = dt_alloc(dtp, len);
>> if (fn == NULL)
>> return -ENOENT;
>>
>> - snprintf(fn, len, "%s" GROUP_FMT "/%s/format",
>> - EVENTSFS, GROUP_DATA, prp->desc->prb);
>> + snprintf(fn, len, "%s" PROBE_FMT "/format",
>> + EVENTSFS, PROBE_DATA);
> How about also changing this to use asprintf() rather than doing the douoble
> snprintf (one to get the langth, the alloc, and then actually populate).
Sure, sounds good. On the other hand, this is in dt_prov_dtrace, while
the main point of the patch is an fbt problem. Yeah, I know, not a big
deal. But since we've been chasing these asprintf() sites
opportunistically over a couple of patches and there are several (4?)
more sites to go, how about I do all these asprintf() conversions in a
separate patch?
>> f = fopen(fn, "r");
>> dt_free(dtp, fn);
>> if (f == NULL)
>> @@ -300,7 +300,7 @@ static void detach(dtrace_hdl_t *dtp, const dt_probe_t *prp)
>> if (fd == -1)
>> return;
>>
>> - dprintf(fd, "-:" GROUP_FMT "/%s\n", GROUP_DATA, prp->desc->prb);
>> + dprintf(fd, "-:" PROBE_FMT "\n", PROBE_DATA);
>> close(fd);
>> }
>>
>> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
>> index 3feac56ea..8dbf9740a 100644
>> --- a/libdtrace/dt_prov_fbt.c
>> +++ b/libdtrace/dt_prov_fbt.c
>> @@ -53,8 +53,7 @@ static const char prvname[] = "fbt";
>>
>> #define KPROBE_EVENTS TRACEFS "kprobe_events"
>>
>> -#define FBT_GROUP_FMT GROUP_FMT "_%s"
>> -#define FBT_GROUP_DATA GROUP_DATA, prp->desc->prb
>> +#define FBT_PROBE_FMT "dt_%d_%s_%s"
>>
>> static const dtrace_pattr_t pattr = {
>> { DTRACE_STABILITY_EVOLVING, DTRACE_STABILITY_EVOLVING, DTRACE_CLASS_COMMON },
>> @@ -508,16 +507,16 @@ static int kprobe_attach(dtrace_hdl_t *dtp, const dt_probe_t *prp, int bpf_fd)
>> if (fd == -1)
>> goto out;
>>
>> - rc = dprintf(fd, "%c:" FBT_GROUP_FMT "/%s %s\n",
>> + rc = dprintf(fd, "%c:" FBT_PROBE_FMT "/%s %s\n",
>> prp->desc->prb[0] == 'e' ? 'p' : 'r',
>> - FBT_GROUP_DATA, tpn, fun);
>> + PROBE_DATA, tpn, fun);
>> close(fd);
>> if (rc == -1)
>> goto out;
>>
>> /* create format file name */
>> - if (asprintf(&fn, "%s" FBT_GROUP_FMT "/%s/format", EVENTSFS,
>> - FBT_GROUP_DATA, tpn) == -1)
>> + if (asprintf(&fn, "%s" FBT_PROBE_FMT "/%s/format", EVENTSFS,
>> + PROBE_DATA, tpn) == -1)
>> goto out;
>>
>> /* open format file */
>> @@ -583,7 +582,7 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp)
>> }
>> }
>>
>> - dprintf(fd, "-:" FBT_GROUP_FMT "/%s\n", FBT_GROUP_DATA, tpn);
>> + dprintf(fd, "-:" FBT_PROBE_FMT "/%s\n", PROBE_DATA, tpn);
>> close(fd);
>>
>> if (tpn != prp->desc->fun)
>> diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c
>> index 709f7040f..e3269f4b3 100644
>> --- a/libdtrace/dt_prov_rawtp.c
>> +++ b/libdtrace/dt_prov_rawtp.c
>> @@ -56,7 +56,7 @@ static const dtrace_pattr_t pattr = {
>> /*
>> * The PROBE_LIST file lists all tracepoints in a <group>:<name> format.
>> * We need to ignore these groups:
>> - * - GROUP_FMT (created by DTrace processes)
>> + * - PROBE_SFMT
>> * - kprobes and uprobes
>> * - syscalls (handled by a different provider)
>> * - pid and usdt probes (ditto)
>> @@ -89,7 +89,7 @@ static int populate(dtrace_hdl_t *dtp)
>>
>> *p++ = '\0';
>>
>> - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) {
>> + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) {
>> free(str);
>> continue;
>> }
>> diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c
>> index 74555f5be..ee2cc3c27 100644
>> --- a/libdtrace/dt_prov_sdt.c
>> +++ b/libdtrace/dt_prov_sdt.c
>> @@ -54,7 +54,7 @@ static const dtrace_pattr_t pattr = {
>> /*
>> * The PROBE_LIST file lists all tracepoints in a <group>:<name> format.
>> * We need to ignore these groups:
>> - * - GROUP_FMT (created by DTrace processes)
>> + * - PROBE_SFMT
>> * - kprobes and uprobes
>> * - syscalls (handled by a different provider)
>> * - pid and usdt probes (ditto)
>> @@ -87,7 +87,7 @@ static int populate(dtrace_hdl_t *dtp)
>>
>> *p++ = '\0';
>>
>> - if (sscanf(buf, GROUP_SFMT, &dummy, &str) == 2) {
>> + if (sscanf(buf, PROBE_SFMT, &dummy, &str) == 2) {
>> free(str);
>> continue;
>> }
>> diff --git a/libdtrace/dt_provider_tp.h b/libdtrace/dt_provider_tp.h
>> index f131b1e02..406609d1b 100644
>> --- a/libdtrace/dt_provider_tp.h
>> +++ b/libdtrace/dt_provider_tp.h
>> @@ -18,13 +18,13 @@ extern "C" {
>> * Tracepoint group naming format for DTrace providers. Providers may append
>> * to this format string as needed.
>> *
>> - * GROUP_DATA provides the necessary data items to populate the format string
>> - * (PID of the dtrace process and the provider name). GROUP_SFMT is like
>> - * GROUP_FMT, but for sscanf().
>> + * PROBE_DATA provides the necessary data items to populate the format string.
>> + * PROBE_FMT formats that data.
>> + * PROBE_SFMT is a format string for recognizing PROBE_FMT data in sscanf().
>> */
>> -#define GROUP_FMT "dt_%d_%s"
>> -#define GROUP_SFMT "dt_%d_%ms"
>> -#define GROUP_DATA getpid(), prvname
>> +#define PROBE_DATA getpid(), prp->desc->prv, prp->desc->prb
>> +#define PROBE_FMT "dt_%d_%s/%s"
>> +#define PROBE_SFMT "dt_%d_%ms"
>>
>> typedef struct tp_probe tp_probe_t;
>>
>> diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.d b/test/unittest/providers/rawfbt/tst.wildcard-provider.d
>> new file mode 100644
>> index 000000000..ad9f4c779
>> --- /dev/null
>> +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.d
>> @@ -0,0 +1,20 @@
>> +/*
>> + * Oracle Linux DTrace.
>> + * Copyright (c) 2026, 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: rawfbt probes can be found even when a wildcard provider
>> + * description also allows fbt probes.
>> + */
>> +
>> +#pragma D option quiet
>> +
>> +BEGIN,
>> +*fbt:vmlinux:do_sys_open*:entry
>> +{
>> + printf("success\n");
>> + exit(0);
>> +}
> I am confused how this test accomplishes verification of the assertion. If
> a regular FBT probe satisfies the probe description, wouldn't this pass also?
> Wouldn't a -l based test be better here, where you can test that the output
> does indeed report both the fbt probe and the rawfbt probe that match?
It appears that -l is not an issue; a -l test does not show the
problem. The test in the patch is derived from the user bug report that
motivated this patch.
The whole prvname problem is that the bad prvname value makes GROUP_DATA
bad, which makes FBT_GROUP_DATA bad, which is an issue only in
kprobe_attach (and detach). So, we need kprobe_attach() to see the problem.
>> diff --git a/test/unittest/providers/rawfbt/tst.wildcard-provider.r b/test/unittest/providers/rawfbt/tst.wildcard-provider.r
>> new file mode 100644
>> index 000000000..b34a52d2f
>> --- /dev/null
>> +++ b/test/unittest/providers/rawfbt/tst.wildcard-provider.r
>> @@ -0,0 +1,2 @@
>> +success
>> +
>> --
>> 2.47.3
>>
More information about the DTrace-devel
mailing list