[PATCH 4/4] rawfbt: selectively allow return() in clauses

[PATCH 4/4] rawfbt: selectively allow return() in clauses

[Kris Van Hees]

The return() action is only allowed in clauses associated with a rawfbt
probe on a function listed in /sys/kernel/debug/error_injexction/list.
Use a reject_clause() callback in the rawfbt provider to enforce this.

The list of allowed function is only initialized the first time it is
needed, and its content will be stored in a hashtable for faster
lookup beyond the first.

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
 libdtrace/dt_prov_fbt.c                       | 134 ++++++++++++++++++
 .../actions/return/err.not_allowed-1.d        |  27 ++++
 .../actions/return/err.not_allowed-1.r        |   2 +
 .../actions/return/err.not_allowed-2.d        |  27 ++++
 .../actions/return/err.not_allowed-2.r        |   2 +
 .../actions/return/err.not_allowed-3.d        |  27 ++++
 .../actions/return/err.not_allowed-3.r        |   2 +
 7 files changed, 221 insertions(+)
 create mode 100644 test/unittest/actions/return/err.not_allowed-1.d
 create mode 100644 test/unittest/actions/return/err.not_allowed-1.r
 create mode 100644 test/unittest/actions/return/err.not_allowed-2.d
 create mode 100644 test/unittest/actions/return/err.not_allowed-2.r
 create mode 100644 test/unittest/actions/return/err.not_allowed-3.d
 create mode 100644 test/unittest/actions/return/err.not_allowed-3.r

diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
index 50fa0d9d..b98d8d87 100644
--- a/libdtrace/dt_prov_fbt.c
+++ b/libdtrace/dt_prov_fbt.c
@@ -591,6 +591,139 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp)
 		free(tpn);
 }
 
+/*
+ * raafbt only:
+ *
+ * Accept all clauses, except those that use the return() action in a function
+ * that does not allow error rejection.
+ */
+#define FUNCS_ALLOW_RETURN	"/sys/kernel/debug/error_injection/list"
+
+typedef struct allowed_fn {
+	const char	*mod;
+	const char	*fun;
+	dt_hentry_t	he;
+} allowed_fun_t;
+
+static uint32_t
+fun_hval(const allowed_fun_t *p)
+{
+	return str2hval(p->fun, p->mod ? str2hval(p->mod, 0) : 0);
+}
+
+static int
+fun_cmp(const allowed_fun_t *p, const allowed_fun_t *q) {
+	int	rc;
+
+	if (p->mod != NULL) {
+		if (q->mod == NULL)
+			return 1;
+		else {
+			rc = strcmp(p->mod, q->mod);
+			if (rc != 0)
+				return rc;
+		}
+	} else if (q->mod != NULL)
+		return -1;
+
+	return strcmp(p->fun, q->fun);
+}
+
+DEFINE_HE_STD_LINK_FUNCS(fun, allowed_fun_t, he)
+
+static void *
+fun_del_entry(allowed_fun_t *head, allowed_fun_t *p)
+{
+	head = fun_del(head, p);
+
+	free((char *)p->mod);
+	free((char *)p->fun);
+	free(p);
+
+	return head;
+}
+
+static dt_htab_ops_t fun_htab_ops = {
+	.hval = (htab_hval_fn)fun_hval,
+	.cmp = (htab_cmp_fn)fun_cmp,
+	.add = (htab_add_fn)fun_add,
+	.del = (htab_del_fn)fun_del_entry,
+	.next = (htab_next_fn)fun_next
+};
+
+static void reject_clause(const dt_probe_t *prp, int clsflags)
+{
+	dt_htab_t	*atab = prp->prov->prv_data;
+	allowed_fun_t	tmpl;
+
+	/* If the clause does not have a return() action, allow it. */
+	if (!(clsflags & DT_CLSFLAG_RETURN))
+		return;
+
+	/*
+	 * If the htab of allowed functions for return() does not exist yet,
+	 * create it.
+	 */
+	if (atab == NULL) {
+		FILE		*f;
+		char		*buf = NULL;
+		size_t		len = 0;
+		allowed_fun_t	*entry;
+
+		atab = dt_htab_create(&fun_htab_ops);
+		if (atab == NULL)
+			return;
+
+		prp->prov->prv_data = atab;
+
+		f = fopen(FUNCS_ALLOW_RETURN, "r");
+		if (f == NULL)
+			return;
+
+		while (getline(&buf, &len, f) >= 0) {
+			char	*p;
+
+			entry = (allowed_fun_t *)malloc(sizeof(allowed_fun_t));
+			if (entry == NULL)
+				break;
+
+			p = strchr(buf, ' ');
+			if (p) {
+				*p++ = '\0';
+				if (*p == '[') {
+					char	*q;
+
+					p++;
+					q = strchr(p, ']');
+					if (q)
+						*q = '\0';
+				} else
+					p = NULL;
+			}
+
+			entry->fun = strdup(buf);
+			entry->mod = p != NULL ? strdup(p) : NULL;
+
+			if (dt_htab_insert(atab, entry) < 0)
+				return;
+		}
+
+		fclose(f);
+	}
+
+	tmpl.mod = prp->desc->mod;
+	tmpl.fun = prp->desc->fun;
+	if (dt_htab_lookup(atab, &tmpl) != NULL)
+		return;
+
+	tmpl.mod = NULL;
+	if (dt_htab_lookup(atab, &tmpl) != NULL)
+		return;
+
+	xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
+		prp->desc->prv, prp->desc->mod, prp->desc->fun, prp->desc->prb);
+}
+
 dt_provimpl_t	dt_fbt_fprobe = {
 	.name		= prvname,
 	.prog_type	= BPF_PROG_TYPE_TRACING,
@@ -628,6 +761,7 @@ dt_provimpl_t	dt_rawfbt = {
 	.populate	= &populate,
 	.provide	= &provide,
 	.load_prog	= &dt_bpf_prog_load,
+	.reject_clause	= &reject_clause,
 	.trampoline	= &kprobe_trampoline,
 	.attach		= &kprobe_attach,
 	.detach		= &kprobe_detach,
diff --git a/test/unittest/actions/return/err.not_allowed-1.d b/test/unittest/actions/return/err.not_allowed-1.d
new file mode 100644
index 00000000..246a68b1
--- /dev/null
+++ b/test/unittest/actions/return/err.not_allowed-1.d
@@ -0,0 +1,27 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2025, 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: return() is not allowed for fbt probes
+ *
+ * SECTION: Actions and Subroutines/return()
+ */
+
+#pragma D option quiet
+#pragma D option destructive
+
+BEGIN
+{
+	ok = 0;
+	exit(0);
+}
+
+fbt:btrfs:open_ctree:entry
+/ok/
+{
+	return(0);
+}
diff --git a/test/unittest/actions/return/err.not_allowed-1.r b/test/unittest/actions/return/err.not_allowed-1.r
new file mode 100644
index 00000000..e3443c30
--- /dev/null
+++ b/test/unittest/actions/return/err.not_allowed-1.r
@@ -0,0 +1,2 @@
+-- @@stderr --
+dtrace: could not enable tracing: return() not allowed for fbt:btrfs:open_ctree:entry
diff --git a/test/unittest/actions/return/err.not_allowed-2.d b/test/unittest/actions/return/err.not_allowed-2.d
new file mode 100644
index 00000000..e0d303c7
--- /dev/null
+++ b/test/unittest/actions/return/err.not_allowed-2.d
@@ -0,0 +1,27 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2025, 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: return() is only allowed for select functions
+ *
+ * SECTION: Actions and Subroutines/return()
+ */
+
+#pragma D option quiet
+#pragma D option destructive
+
+BEGIN
+{
+	ok = 0;
+	exit(0);
+}
+
+rawfbt:btrfs:close_ctree:entry
+/ok/
+{
+	return(0);
+}
diff --git a/test/unittest/actions/return/err.not_allowed-2.r b/test/unittest/actions/return/err.not_allowed-2.r
new file mode 100644
index 00000000..758d3477
--- /dev/null
+++ b/test/unittest/actions/return/err.not_allowed-2.r
@@ -0,0 +1,2 @@
+-- @@stderr --
+dtrace: could not enable tracing: return() not allowed for rawfbt:btrfs:close_ctree:entry
diff --git a/test/unittest/actions/return/err.not_allowed-3.d b/test/unittest/actions/return/err.not_allowed-3.d
new file mode 100644
index 00000000..770565c9
--- /dev/null
+++ b/test/unittest/actions/return/err.not_allowed-3.d
@@ -0,0 +1,27 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2025, 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: return() is only allowed for select functions
+ *
+ * SECTION: Actions and Subroutines/return()
+ */
+
+#pragma D option quiet
+#pragma D option destructive
+
+BEGIN
+{
+	ok = 0;
+	exit(0);
+}
+
+rawfbt:btrfs:*_ctree:entry
+/ok/
+{
+	return(0);
+}
diff --git a/test/unittest/actions/return/err.not_allowed-3.r b/test/unittest/actions/return/err.not_allowed-3.r
new file mode 100644
index 00000000..758d3477
--- /dev/null
+++ b/test/unittest/actions/return/err.not_allowed-3.r
@@ -0,0 +1,2 @@
+-- @@stderr --
+dtrace: could not enable tracing: return() not allowed for rawfbt:btrfs:close_ctree:entry
-- 
2.43.5

Re: [DTrace-devel] [PATCH 4/4] rawfbt: selectively allow return() in clauses

[Nick Alcock]

On 15 Jul 2025, Kris Van Hees via DTrace-devel verbalised:

> The return() action is only allowed in clauses associated with a rawfbt
> probe on a function listed in /sys/kernel/debug/error_injexction/list.
> Use a reject_clause() callback in the rawfbt provider to enforce this.
>
> The list of allowed function is only initialized the first time it is
> needed, and its content will be stored in a hashtable for faster
> lookup beyond the first.

Pluralize "function".

> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>

Reviewed-by: Nick Alcock <nick.alcock@oracle.com>

modulo typos and other nits above and below. Nothing serious found, just
misconceptions of mine being squashed.

> +/*
> + * raafbt only:

rawfbt

> + *
> + * Accept all clauses, except those that use the return() action in a function
> + * that does not allow error rejection.
> + */
> +#define FUNCS_ALLOW_RETURN	"/sys/kernel/debug/error_injection/list"
> +
> +typedef struct allowed_fn {
> +	const char	*mod;
> +	const char	*fun;
> +	dt_hentry_t	he;
> +} allowed_fun_t;
> +
> +static uint32_t
> +fun_hval(const allowed_fun_t *p)
> +{
> +	return str2hval(p->fun, p->mod ? str2hval(p->mod, 0) : 0);
> +}
> +
> +static int
> +fun_cmp(const allowed_fun_t *p, const allowed_fun_t *q) {
> +	int	rc;
> +
> +	if (p->mod != NULL) {
> +		if (q->mod == NULL)
> +			return 1;
> +		else {
> +			rc = strcmp(p->mod, q->mod);
> +			if (rc != 0)
> +				return rc;
> +		}
> +	} else if (q->mod != NULL)
> +		return -1;

Ugh, that's quite tangled for how little it does :( can't think of a
nicer approach though.

> +static void reject_clause(const dt_probe_t *prp, int clsflags)
> +{
> +	dt_htab_t	*atab = prp->prov->prv_data;
> +	allowed_fun_t	tmpl;
> +
> +	/* If the clause does not have a return() action, allow it. */
> +	if (!(clsflags & DT_CLSFLAG_RETURN))
> +		return;
> +
> +	/*
> +	 * If the htab of allowed functions for return() does not exist yet,
> +	 * create it.
> +	 */

Personally I'd move this into a function just to do this initialization,
and call it from this one.

> +	if (atab == NULL) {
> +		FILE		*f;
> +		char		*buf = NULL;
> +		size_t		len = 0;
> +		allowed_fun_t	*entry;
> +
> +		atab = dt_htab_create(&fun_htab_ops);
> +		if (atab == NULL)
> +			return;
> +
> +		prp->prov->prv_data = atab;
> +
> +		f = fopen(FUNCS_ALLOW_RETURN, "r");
> +		if (f == NULL)
> +			return;

Might want to at least dt_dprintf something here, or more likely
xyerror() out, given that this probably indicates a kernel configuration
problem. Right now this looks exactly like a success (!).

> +		while (getline(&buf, &len, f) >= 0) {
> +			char	*p;
> +
> +			entry = (allowed_fun_t *)malloc(sizeof(allowed_fun_t));
> +			if (entry == NULL)
> +				break;
> +
> +			p = strchr(buf, ' ');
> +			if (p) {
> +				*p++ = '\0';
> +				if (*p == '[') {
> +					char	*q;
> +
> +					p++;
> +					q = strchr(p, ']');
> +					if (q)
> +						*q = '\0';
> +				} else
> +					p = NULL;
> +			}
> +
> +			entry->fun = strdup(buf);
> +			entry->mod = p != NULL ? strdup(p) : NULL;
> +
> +			if (dt_htab_insert(atab, entry) < 0)
> +				return;
> +		}
> +
> +		fclose(f);

free(buf);

> +	}
> +
> +	tmpl.mod = prp->desc->mod;
> +	tmpl.fun = prp->desc->fun;
> +	if (dt_htab_lookup(atab, &tmpl) != NULL)
> +		return;
> +
> +	tmpl.mod = NULL;
> +	if (dt_htab_lookup(atab, &tmpl) != NULL)
> +		return;
> +
> +	xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
> +		prp->desc->prv, prp->desc->mod, prp->desc->fun, prp->desc->prb);
> +}

(end of function for comparision with "error" path above...)

>  dt_provimpl_t	dt_fbt_fprobe = {
>  	.name		= prvname,
>  	.prog_type	= BPF_PROG_TYPE_TRACING,
> @@ -628,6 +761,7 @@ dt_provimpl_t	dt_rawfbt = {
>  	.populate	= &populate,
>  	.provide	= &provide,
>  	.load_prog	= &dt_bpf_prog_load,
> +	.reject_clause	= &reject_clause,
>  	.trampoline	= &kprobe_trampoline,
>  	.attach		= &kprobe_attach,
>  	.detach		= &kprobe_detach,

So unlike what I assumed in the previous commit, we're assigning to
reject_clause() whether or not destructive tracing is on...

... because dt_cg_clsflags() is turning on the destructive flag whenever
the return flag is on. OK.

> diff --git a/test/unittest/actions/return/err.not_allowed-1.d b/test/unittest/actions/return/err.not_allowed-1.d
> new file mode 100644
> index 00000000..246a68b1
> --- /dev/null
> +++ b/test/unittest/actions/return/err.not_allowed-1.d
> @@ -0,0 +1,27 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2025, 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: return() is not allowed for fbt probes
> + *
> + * SECTION: Actions and Subroutines/return()
> + */
> +
> +#pragma D option quiet
> +#pragma D option destructive
> +
> +BEGIN
> +{
> +	ok = 0;
> +	exit(0);
> +}
> +
> +fbt:btrfs:open_ctree:entry
> +/ok/
> +{
> +	return(0);
> +}

This seems like code that will rot fast, given that btrfs is one of the
few modules that actually does have error injection. Why not pick some
of the huge mass of the kernel that doesn't have it at all? (I guess the
rot will be easy to see -- new test failures, every time...)

-- 
NULL && (void)

Re: [DTrace-devel] [PATCH 4/4] rawfbt: selectively allow return() in clauses

[Kris Van Hees]

On Tue, Jul 15, 2025 at 11:50:21AM +0100, Nick Alcock wrote:
> On 15 Jul 2025, Kris Van Hees via DTrace-devel verbalised:
> 
> > The return() action is only allowed in clauses associated with a rawfbt
> > probe on a function listed in /sys/kernel/debug/error_injexction/list.
> > Use a reject_clause() callback in the rawfbt provider to enforce this.
> >
> > The list of allowed function is only initialized the first time it is
> > needed, and its content will be stored in a hashtable for faster
> > lookup beyond the first.
> 
> Pluralize "function".
> 
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> 
> Reviewed-by: Nick Alcock <nick.alcock@oracle.com>
> 
> modulo typos and other nits above and below. Nothing serious found, just
> misconceptions of mine being squashed.
> 
> > +/*
> > + * raafbt only:
> 
> rawfbt
> 
> > + *
> > + * Accept all clauses, except those that use the return() action in a function
> > + * that does not allow error rejection.
> > + */
> > +#define FUNCS_ALLOW_RETURN	"/sys/kernel/debug/error_injection/list"
> > +
> > +typedef struct allowed_fn {
> > +	const char	*mod;
> > +	const char	*fun;
> > +	dt_hentry_t	he;
> > +} allowed_fun_t;
> > +
> > +static uint32_t
> > +fun_hval(const allowed_fun_t *p)
> > +{
> > +	return str2hval(p->fun, p->mod ? str2hval(p->mod, 0) : 0);
> > +}
> > +
> > +static int
> > +fun_cmp(const allowed_fun_t *p, const allowed_fun_t *q) {
> > +	int	rc;
> > +
> > +	if (p->mod != NULL) {
> > +		if (q->mod == NULL)
> > +			return 1;
> > +		else {
> > +			rc = strcmp(p->mod, q->mod);
> > +			if (rc != 0)
> > +				return rc;
> > +		}
> > +	} else if (q->mod != NULL)
> > +		return -1;
> 
> Ugh, that's quite tangled for how little it does :( can't think of a
> nicer approach though.

I am hoping that perhaps in time a lot more functions may allow this, plus if
someone is going to make use of this feature for error injection they are
likely to use it for multiple functions and then being able to use a hashtable
rather than scanning the list multiple times is nicer.

> > +static void reject_clause(const dt_probe_t *prp, int clsflags)
> > +{
> > +	dt_htab_t	*atab = prp->prov->prv_data;
> > +	allowed_fun_t	tmpl;
> > +
> > +	/* If the clause does not have a return() action, allow it. */
> > +	if (!(clsflags & DT_CLSFLAG_RETURN))
> > +		return;
> > +
> > +	/*
> > +	 * If the htab of allowed functions for return() does not exist yet,
> > +	 * create it.
> > +	 */
> 
> Personally I'd move this into a function just to do this initialization,
> and call it from this one.
> 
> > +	if (atab == NULL) {
> > +		FILE		*f;
> > +		char		*buf = NULL;
> > +		size_t		len = 0;
> > +		allowed_fun_t	*entry;
> > +
> > +		atab = dt_htab_create(&fun_htab_ops);
> > +		if (atab == NULL)
> > +			return;
> > +
> > +		prp->prov->prv_data = atab;
> > +
> > +		f = fopen(FUNCS_ALLOW_RETURN, "r");
> > +		if (f == NULL)
> > +			return;
> 
> Might want to at least dt_dprintf something here, or more likely
> xyerror() out, given that this probably indicates a kernel configuration
> problem. Right now this looks exactly like a success (!).

Hm, ah yes, I did this before I added the clsflags test before, so yes, if
we cannot read the file, then we should also flag a compilation error because
it means that error injection is not configured in the kernel (or for some
other reason won't be possible).

Will fix.

> > +		while (getline(&buf, &len, f) >= 0) {
> > +			char	*p;
> > +
> > +			entry = (allowed_fun_t *)malloc(sizeof(allowed_fun_t));
> > +			if (entry == NULL)
> > +				break;
> > +
> > +			p = strchr(buf, ' ');
> > +			if (p) {
> > +				*p++ = '\0';
> > +				if (*p == '[') {
> > +					char	*q;
> > +
> > +					p++;
> > +					q = strchr(p, ']');
> > +					if (q)
> > +						*q = '\0';
> > +				} else
> > +					p = NULL;
> > +			}
> > +
> > +			entry->fun = strdup(buf);
> > +			entry->mod = p != NULL ? strdup(p) : NULL;
> > +
> > +			if (dt_htab_insert(atab, entry) < 0)
> > +				return;
> > +		}
> > +
> > +		fclose(f);
> 
> free(buf);

Oops, thanks!

> > +	}
> > +
> > +	tmpl.mod = prp->desc->mod;
> > +	tmpl.fun = prp->desc->fun;
> > +	if (dt_htab_lookup(atab, &tmpl) != NULL)
> > +		return;
> > +
> > +	tmpl.mod = NULL;
> > +	if (dt_htab_lookup(atab, &tmpl) != NULL)
> > +		return;
> > +
> > +	xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
> > +		prp->desc->prv, prp->desc->mod, prp->desc->fun, prp->desc->prb);
> > +}
> 
> (end of function for comparision with "error" path above...)

Yes, indeed.  (See comment above.)

> >  dt_provimpl_t	dt_fbt_fprobe = {
> >  	.name		= prvname,
> >  	.prog_type	= BPF_PROG_TYPE_TRACING,
> > @@ -628,6 +761,7 @@ dt_provimpl_t	dt_rawfbt = {
> >  	.populate	= &populate,
> >  	.provide	= &provide,
> >  	.load_prog	= &dt_bpf_prog_load,
> > +	.reject_clause	= &reject_clause,
> >  	.trampoline	= &kprobe_trampoline,
> >  	.attach		= &kprobe_attach,
> >  	.detach		= &kprobe_detach,
> 
> So unlike what I assumed in the previous commit, we're assigning to
> reject_clause() whether or not destructive tracing is on...
> 
> ... because dt_cg_clsflags() is turning on the destructive flag whenever
> the return flag is on. OK.
> 
> > diff --git a/test/unittest/actions/return/err.not_allowed-1.d b/test/unittest/actions/return/err.not_allowed-1.d
> > new file mode 100644
> > index 00000000..246a68b1
> > --- /dev/null
> > +++ b/test/unittest/actions/return/err.not_allowed-1.d
> > @@ -0,0 +1,27 @@
> > +/*
> > + * Oracle Linux DTrace.
> > + * Copyright (c) 2025, 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: return() is not allowed for fbt probes
> > + *
> > + * SECTION: Actions and Subroutines/return()
> > + */
> > +
> > +#pragma D option quiet
> > +#pragma D option destructive
> > +
> > +BEGIN
> > +{
> > +	ok = 0;
> > +	exit(0);
> > +}
> > +
> > +fbt:btrfs:open_ctree:entry
> > +/ok/
> > +{
> > +	return(0);
> > +}
> 
> This seems like code that will rot fast, given that btrfs is one of the
> few modules that actually does have error injection. Why not pick some
> of the huge mass of the kernel that doesn't have it at all? (I guess the
> rot will be easy to see -- new test failures, every time...)

I used this probe because the other tests for error injection use it as well,
so at least if there is rot, we'll notice it real quick across multiple
related tests.

Re: [DTrace-devel] [PATCH 4/4] rawfbt: selectively allow return() in clauses

[Eugene Loh]

[-- Attachment #1: Type: text/plain, Size: 9311 bytes --]

I'm late to the party, sorry.

For usability, could an error message point to 
/sys/kernel/debug/error_injection/list?  Might as well provide useful 
information at the time of encounter.

When you parse the list, you expect the function name to be terminated 
with a space.  When I look at my list, only 7 (out of nearly 1000) are 
so terminated.  Shouldn't you check for a space *OR* a tab?

When I run the tests, I get an OL8 UEK6 failure on x86 for 
test/unittest/actions/return/tst.destructive.d:
dtrace: could not enable tracing: Failed to enable 
rawfbt:btrfs:open_ctree:entry: No such file or directory

Also, there should be a test to check that return() actually does the 
right thing.  I'm attaching a proposal.

Typos:  "injexction" in the commit msg and raa as Nick previously noted.

On 7/15/25 01:48, Kris Van Hees via DTrace-devel wrote:

> The return() action is only allowed in clauses associated with a rawfbt
> probe on a function listed in /sys/kernel/debug/error_injexction/list.
> Use a reject_clause() callback in the rawfbt provider to enforce this.
>
> The list of allowed function is only initialized the first time it is
> needed, and its content will be stored in a hashtable for faster
> lookup beyond the first.
>
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>   libdtrace/dt_prov_fbt.c                       | 134 ++++++++++++++++++
>   .../actions/return/err.not_allowed-1.d        |  27 ++++
>   .../actions/return/err.not_allowed-1.r        |   2 +
>   .../actions/return/err.not_allowed-2.d        |  27 ++++
>   .../actions/return/err.not_allowed-2.r        |   2 +
>   .../actions/return/err.not_allowed-3.d        |  27 ++++
>   .../actions/return/err.not_allowed-3.r        |   2 +
>   7 files changed, 221 insertions(+)
>   create mode 100644 test/unittest/actions/return/err.not_allowed-1.d
>   create mode 100644 test/unittest/actions/return/err.not_allowed-1.r
>   create mode 100644 test/unittest/actions/return/err.not_allowed-2.d
>   create mode 100644 test/unittest/actions/return/err.not_allowed-2.r
>   create mode 100644 test/unittest/actions/return/err.not_allowed-3.d
>   create mode 100644 test/unittest/actions/return/err.not_allowed-3.r
>
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 50fa0d9d..b98d8d87 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -591,6 +591,139 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp)
>   		free(tpn);
>   }
>   
> +/*
> + * raafbt only:
> + *
> + * Accept all clauses, except those that use the return() action in a function
> + * that does not allow error rejection.
> + */
> +#define FUNCS_ALLOW_RETURN	"/sys/kernel/debug/error_injection/list"
> +
> +typedef struct allowed_fn {
> +	const char	*mod;
> +	const char	*fun;
> +	dt_hentry_t	he;
> +} allowed_fun_t;
> +
> +static uint32_t
> +fun_hval(const allowed_fun_t *p)
> +{
> +	return str2hval(p->fun, p->mod ? str2hval(p->mod, 0) : 0);
> +}
> +
> +static int
> +fun_cmp(const allowed_fun_t *p, const allowed_fun_t *q) {
> +	int	rc;
> +
> +	if (p->mod != NULL) {
> +		if (q->mod == NULL)
> +			return 1;
> +		else {
> +			rc = strcmp(p->mod, q->mod);
> +			if (rc != 0)
> +				return rc;
> +		}
> +	} else if (q->mod != NULL)
> +		return -1;
> +
> +	return strcmp(p->fun, q->fun);
> +}
> +
> +DEFINE_HE_STD_LINK_FUNCS(fun, allowed_fun_t, he)
> +
> +static void *
> +fun_del_entry(allowed_fun_t *head, allowed_fun_t *p)
> +{
> +	head = fun_del(head, p);
> +
> +	free((char *)p->mod);
> +	free((char *)p->fun);
> +	free(p);
> +
> +	return head;
> +}
> +
> +static dt_htab_ops_t fun_htab_ops = {
> +	.hval = (htab_hval_fn)fun_hval,
> +	.cmp = (htab_cmp_fn)fun_cmp,
> +	.add = (htab_add_fn)fun_add,
> +	.del = (htab_del_fn)fun_del_entry,
> +	.next = (htab_next_fn)fun_next
> +};
> +
> +static void reject_clause(const dt_probe_t *prp, int clsflags)
> +{
> +	dt_htab_t	*atab = prp->prov->prv_data;
> +	allowed_fun_t	tmpl;
> +
> +	/* If the clause does not have a return() action, allow it. */
> +	if (!(clsflags & DT_CLSFLAG_RETURN))
> +		return;
> +
> +	/*
> +	 * If the htab of allowed functions for return() does not exist yet,
> +	 * create it.
> +	 */
> +	if (atab == NULL) {
> +		FILE		*f;
> +		char		*buf = NULL;
> +		size_t		len = 0;
> +		allowed_fun_t	*entry;
> +
> +		atab = dt_htab_create(&fun_htab_ops);
> +		if (atab == NULL)
> +			return;
> +
> +		prp->prov->prv_data = atab;
> +
> +		f = fopen(FUNCS_ALLOW_RETURN, "r");
> +		if (f == NULL)
> +			return;
> +
> +		while (getline(&buf, &len, f) >= 0) {
> +			char	*p;
> +
> +			entry = (allowed_fun_t *)malloc(sizeof(allowed_fun_t));
> +			if (entry == NULL)
> +				break;
> +
> +			p = strchr(buf, ' ');
> +			if (p) {
> +				*p++ = '\0';
> +				if (*p == '[') {
> +					char	*q;
> +
> +					p++;
> +					q = strchr(p, ']');
> +					if (q)
> +						*q = '\0';
> +				} else
> +					p = NULL;
> +			}
> +
> +			entry->fun = strdup(buf);
> +			entry->mod = p != NULL ? strdup(p) : NULL;
> +
> +			if (dt_htab_insert(atab, entry) < 0)
> +				return;
> +		}
> +
> +		fclose(f);
> +	}
> +
> +	tmpl.mod = prp->desc->mod;
> +	tmpl.fun = prp->desc->fun;
> +	if (dt_htab_lookup(atab, &tmpl) != NULL)
> +		return;
> +
> +	tmpl.mod = NULL;
> +	if (dt_htab_lookup(atab, &tmpl) != NULL)
> +		return;
> +
> +	xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
> +		prp->desc->prv, prp->desc->mod, prp->desc->fun, prp->desc->prb);
> +}
> +
>   dt_provimpl_t	dt_fbt_fprobe = {
>   	.name		= prvname,
>   	.prog_type	= BPF_PROG_TYPE_TRACING,
> @@ -628,6 +761,7 @@ dt_provimpl_t	dt_rawfbt = {
>   	.populate	= &populate,
>   	.provide	= &provide,
>   	.load_prog	= &dt_bpf_prog_load,
> +	.reject_clause	= &reject_clause,
>   	.trampoline	= &kprobe_trampoline,
>   	.attach		= &kprobe_attach,
>   	.detach		= &kprobe_detach,
> diff --git a/test/unittest/actions/return/err.not_allowed-1.d b/test/unittest/actions/return/err.not_allowed-1.d
> new file mode 100644
> index 00000000..246a68b1
> --- /dev/null
> +++ b/test/unittest/actions/return/err.not_allowed-1.d
> @@ -0,0 +1,27 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2025, 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: return() is not allowed for fbt probes
> + *
> + * SECTION: Actions and Subroutines/return()
> + */
> +
> +#pragma D option quiet
> +#pragma D option destructive
> +
> +BEGIN
> +{
> +	ok = 0;
> +	exit(0);
> +}
> +
> +fbt:btrfs:open_ctree:entry
> +/ok/
> +{
> +	return(0);
> +}
> diff --git a/test/unittest/actions/return/err.not_allowed-1.r b/test/unittest/actions/return/err.not_allowed-1.r
> new file mode 100644
> index 00000000..e3443c30
> --- /dev/null
> +++ b/test/unittest/actions/return/err.not_allowed-1.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: could not enable tracing: return() not allowed for fbt:btrfs:open_ctree:entry
> diff --git a/test/unittest/actions/return/err.not_allowed-2.d b/test/unittest/actions/return/err.not_allowed-2.d
> new file mode 100644
> index 00000000..e0d303c7
> --- /dev/null
> +++ b/test/unittest/actions/return/err.not_allowed-2.d
> @@ -0,0 +1,27 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2025, 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: return() is only allowed for select functions
> + *
> + * SECTION: Actions and Subroutines/return()
> + */
> +
> +#pragma D option quiet
> +#pragma D option destructive
> +
> +BEGIN
> +{
> +	ok = 0;
> +	exit(0);
> +}
> +
> +rawfbt:btrfs:close_ctree:entry
> +/ok/
> +{
> +	return(0);
> +}
> diff --git a/test/unittest/actions/return/err.not_allowed-2.r b/test/unittest/actions/return/err.not_allowed-2.r
> new file mode 100644
> index 00000000..758d3477
> --- /dev/null
> +++ b/test/unittest/actions/return/err.not_allowed-2.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: could not enable tracing: return() not allowed for rawfbt:btrfs:close_ctree:entry
> diff --git a/test/unittest/actions/return/err.not_allowed-3.d b/test/unittest/actions/return/err.not_allowed-3.d
> new file mode 100644
> index 00000000..770565c9
> --- /dev/null
> +++ b/test/unittest/actions/return/err.not_allowed-3.d
> @@ -0,0 +1,27 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2025, 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: return() is only allowed for select functions
> + *
> + * SECTION: Actions and Subroutines/return()
> + */
> +
> +#pragma D option quiet
> +#pragma D option destructive
> +
> +BEGIN
> +{
> +	ok = 0;
> +	exit(0);
> +}
> +
> +rawfbt:btrfs:*_ctree:entry
> +/ok/
> +{
> +	return(0);
> +}
> diff --git a/test/unittest/actions/return/err.not_allowed-3.r b/test/unittest/actions/return/err.not_allowed-3.r
> new file mode 100644
> index 00000000..758d3477
> --- /dev/null
> +++ b/test/unittest/actions/return/err.not_allowed-3.r
> @@ -0,0 +1,2 @@
> +-- @@stderr --
> +dtrace: could not enable tracing: return() not allowed for rawfbt:btrfs:close_ctree:entry

[-- Attachment #2: 0001-test.patch --]
[-- Type: text/plain, Size: 2776 bytes --]

From a6a6ceb614fadd5c5d1116cc05fe71da2b2dbff7 Mon Sep 17 00:00:00 2001
From: Eugene Loh <eugene.loh@oracle.com>
Date: Tue, 15 Jul 2025 13:33:04 -0700
Subject: [PATCH] test

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 .../actions/return/tst.destructive-getpid.r   | 12 +++++
 .../actions/return/tst.destructive-getpid.r.p | 10 ++++
 .../actions/return/tst.destructive-getpid.sh  | 48 +++++++++++++++++++
 3 files changed, 70 insertions(+)
 create mode 100644 test/unittest/actions/return/tst.destructive-getpid.r
 create mode 100755 test/unittest/actions/return/tst.destructive-getpid.r.p
 create mode 100755 test/unittest/actions/return/tst.destructive-getpid.sh

diff --git a/test/unittest/actions/return/tst.destructive-getpid.r b/test/unittest/actions/return/tst.destructive-getpid.r
new file mode 100644
index 00000000..b6bdf211
--- /dev/null
+++ b/test/unittest/actions/return/tst.destructive-getpid.r
@@ -0,0 +1,12 @@
+pid is mypid
+mypid
+22222
+mypid
+mypid
+55555
+mypid
+mypid
+mypid
+mypid
+mypid
+
diff --git a/test/unittest/actions/return/tst.destructive-getpid.r.p b/test/unittest/actions/return/tst.destructive-getpid.r.p
new file mode 100755
index 00000000..d6593c46
--- /dev/null
+++ b/test/unittest/actions/return/tst.destructive-getpid.r.p
@@ -0,0 +1,10 @@
+#!/usr/bin/gawk -f
+
+BEGIN { mypid = -1 }
+
+/^pid is [1-9][0-9]*$/ { mypid = $3; }
+
+{
+        if ( $NF == mypid ) $NF = "mypid";
+        print;
+}
diff --git a/test/unittest/actions/return/tst.destructive-getpid.sh b/test/unittest/actions/return/tst.destructive-getpid.sh
new file mode 100755
index 00000000..7c44dde9
--- /dev/null
+++ b/test/unittest/actions/return/tst.destructive-getpid.sh
@@ -0,0 +1,48 @@
+#!/bin/bash
+#
+# Oracle Linux DTrace.
+# Copyright (c) 2025, 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.
+#
+
+dtrace=$1
+
+# Set up test directory.
+
+DIRNAME=$tmpdir/destructive-getpid.$$.$RANDOM
+mkdir -p $DIRNAME
+cd $DIRNAME
+
+# Make the trigger.  It reports the pid 10 times.
+
+cat << EOF > main.c
+#include <stdio.h>
+#include <unistd.h>
+
+int main(int c, char **v) {
+	int i;
+
+	for (i = 0; i < 10; i++)
+		printf("%d\n", getpid());
+
+	return 0;
+}
+EOF
+
+$CC main.c
+if [ $? -ne 0 ]; then
+	echo ERROR: compile
+	exit 1
+fi
+
+# Trace the trigger.  On the 2nd and 5th getpid() calls, modify the result.
+
+$dtrace -c ./a.out -w -q -n '
+BEGIN { printf("pid is %d\n", $target); n = 0; }
+rawfbt:vmlinux:__*_sys_getpid:entry /pid == $target/ { n++ }
+rawfbt:vmlinux:__*_sys_getpid:entry /pid == $target && n == 2/ { return(22222); }
+rawfbt:vmlinux:__*_sys_getpid:entry /pid == $target && n == 5/ { return(55555); }
+'
+
+exit $?
-- 
2.18.4

Re: [DTrace-devel] [PATCH 4/4] rawfbt: selectively allow return() in clauses

[Kris Van Hees]

On Tue, Jul 15, 2025 at 05:22:42PM -0400, Eugene Loh wrote:
> I'm late to the party, sorry.
> 
> For usability, could an error message point to
> /sys/kernel/debug/error_injection/list?  Might as well provide useful
> information at the time of encounter.

Well, yes and no.  Compilation errors rarely print out documentation.  Pointing
at that list is not really useful unless you also explain what it means etc.
Which becomes too verbose.  Honestly, this is a feature that should not be
used by someone who does not know already what they are playing with.

> When you parse the list, you expect the function name to be terminated with
> a space.  When I look at my list, only 7 (out of nearly 1000) are so
> terminated.  Shouldn't you check for a space *OR* a tab?

Ah yes, fixing.

> When I run the tests, I get an OL8 UEK6 failure on x86 for
> test/unittest/actions/return/tst.destructive.d:
> dtrace: could not enable tracing: Failed to enable
> rawfbt:btrfs:open_ctree:entry: No such file or directory

Hm, I'll have to look into taht - I expect that it was not available in UEK6
(or the config wasn't enabled).

> Also, there should be a test to check that return() actually does the right
> thing.  I'm attaching a proposal.

Let me look at that - Thanks for writing one!  I'll check it out and almost
certainly include it.

> Typos:  "injexction" in the commit msg and raa as Nick previously noted.

Fixed.

> On 7/15/25 01:48, Kris Van Hees via DTrace-devel wrote:
> 
> > The return() action is only allowed in clauses associated with a rawfbt
> > probe on a function listed in /sys/kernel/debug/error_injexction/list.
> > Use a reject_clause() callback in the rawfbt provider to enforce this.
> > 
> > The list of allowed function is only initialized the first time it is
> > needed, and its content will be stored in a hashtable for faster
> > lookup beyond the first.
> > 
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> >   libdtrace/dt_prov_fbt.c                       | 134 ++++++++++++++++++
> >   .../actions/return/err.not_allowed-1.d        |  27 ++++
> >   .../actions/return/err.not_allowed-1.r        |   2 +
> >   .../actions/return/err.not_allowed-2.d        |  27 ++++
> >   .../actions/return/err.not_allowed-2.r        |   2 +
> >   .../actions/return/err.not_allowed-3.d        |  27 ++++
> >   .../actions/return/err.not_allowed-3.r        |   2 +
> >   7 files changed, 221 insertions(+)
> >   create mode 100644 test/unittest/actions/return/err.not_allowed-1.d
> >   create mode 100644 test/unittest/actions/return/err.not_allowed-1.r
> >   create mode 100644 test/unittest/actions/return/err.not_allowed-2.d
> >   create mode 100644 test/unittest/actions/return/err.not_allowed-2.r
> >   create mode 100644 test/unittest/actions/return/err.not_allowed-3.d
> >   create mode 100644 test/unittest/actions/return/err.not_allowed-3.r
> > 
> > diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> > index 50fa0d9d..b98d8d87 100644
> > --- a/libdtrace/dt_prov_fbt.c
> > +++ b/libdtrace/dt_prov_fbt.c
> > @@ -591,6 +591,139 @@ static void kprobe_detach(dtrace_hdl_t *dtp, const dt_probe_t *prp)
> >   		free(tpn);
> >   }
> > +/*
> > + * raafbt only:
> > + *
> > + * Accept all clauses, except those that use the return() action in a function
> > + * that does not allow error rejection.
> > + */
> > +#define FUNCS_ALLOW_RETURN	"/sys/kernel/debug/error_injection/list"
> > +
> > +typedef struct allowed_fn {
> > +	const char	*mod;
> > +	const char	*fun;
> > +	dt_hentry_t	he;
> > +} allowed_fun_t;
> > +
> > +static uint32_t
> > +fun_hval(const allowed_fun_t *p)
> > +{
> > +	return str2hval(p->fun, p->mod ? str2hval(p->mod, 0) : 0);
> > +}
> > +
> > +static int
> > +fun_cmp(const allowed_fun_t *p, const allowed_fun_t *q) {
> > +	int	rc;
> > +
> > +	if (p->mod != NULL) {
> > +		if (q->mod == NULL)
> > +			return 1;
> > +		else {
> > +			rc = strcmp(p->mod, q->mod);
> > +			if (rc != 0)
> > +				return rc;
> > +		}
> > +	} else if (q->mod != NULL)
> > +		return -1;
> > +
> > +	return strcmp(p->fun, q->fun);
> > +}
> > +
> > +DEFINE_HE_STD_LINK_FUNCS(fun, allowed_fun_t, he)
> > +
> > +static void *
> > +fun_del_entry(allowed_fun_t *head, allowed_fun_t *p)
> > +{
> > +	head = fun_del(head, p);
> > +
> > +	free((char *)p->mod);
> > +	free((char *)p->fun);
> > +	free(p);
> > +
> > +	return head;
> > +}
> > +
> > +static dt_htab_ops_t fun_htab_ops = {
> > +	.hval = (htab_hval_fn)fun_hval,
> > +	.cmp = (htab_cmp_fn)fun_cmp,
> > +	.add = (htab_add_fn)fun_add,
> > +	.del = (htab_del_fn)fun_del_entry,
> > +	.next = (htab_next_fn)fun_next
> > +};
> > +
> > +static void reject_clause(const dt_probe_t *prp, int clsflags)
> > +{
> > +	dt_htab_t	*atab = prp->prov->prv_data;
> > +	allowed_fun_t	tmpl;
> > +
> > +	/* If the clause does not have a return() action, allow it. */
> > +	if (!(clsflags & DT_CLSFLAG_RETURN))
> > +		return;
> > +
> > +	/*
> > +	 * If the htab of allowed functions for return() does not exist yet,
> > +	 * create it.
> > +	 */
> > +	if (atab == NULL) {
> > +		FILE		*f;
> > +		char		*buf = NULL;
> > +		size_t		len = 0;
> > +		allowed_fun_t	*entry;
> > +
> > +		atab = dt_htab_create(&fun_htab_ops);
> > +		if (atab == NULL)
> > +			return;
> > +
> > +		prp->prov->prv_data = atab;
> > +
> > +		f = fopen(FUNCS_ALLOW_RETURN, "r");
> > +		if (f == NULL)
> > +			return;
> > +
> > +		while (getline(&buf, &len, f) >= 0) {
> > +			char	*p;
> > +
> > +			entry = (allowed_fun_t *)malloc(sizeof(allowed_fun_t));
> > +			if (entry == NULL)
> > +				break;
> > +
> > +			p = strchr(buf, ' ');
> > +			if (p) {
> > +				*p++ = '\0';
> > +				if (*p == '[') {
> > +					char	*q;
> > +
> > +					p++;
> > +					q = strchr(p, ']');
> > +					if (q)
> > +						*q = '\0';
> > +				} else
> > +					p = NULL;
> > +			}
> > +
> > +			entry->fun = strdup(buf);
> > +			entry->mod = p != NULL ? strdup(p) : NULL;
> > +
> > +			if (dt_htab_insert(atab, entry) < 0)
> > +				return;
> > +		}
> > +
> > +		fclose(f);
> > +	}
> > +
> > +	tmpl.mod = prp->desc->mod;
> > +	tmpl.fun = prp->desc->fun;
> > +	if (dt_htab_lookup(atab, &tmpl) != NULL)
> > +		return;
> > +
> > +	tmpl.mod = NULL;
> > +	if (dt_htab_lookup(atab, &tmpl) != NULL)
> > +		return;
> > +
> > +	xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
> > +		prp->desc->prv, prp->desc->mod, prp->desc->fun, prp->desc->prb);
> > +}
> > +
> >   dt_provimpl_t	dt_fbt_fprobe = {
> >   	.name		= prvname,
> >   	.prog_type	= BPF_PROG_TYPE_TRACING,
> > @@ -628,6 +761,7 @@ dt_provimpl_t	dt_rawfbt = {
> >   	.populate	= &populate,
> >   	.provide	= &provide,
> >   	.load_prog	= &dt_bpf_prog_load,
> > +	.reject_clause	= &reject_clause,
> >   	.trampoline	= &kprobe_trampoline,
> >   	.attach		= &kprobe_attach,
> >   	.detach		= &kprobe_detach,
> > diff --git a/test/unittest/actions/return/err.not_allowed-1.d b/test/unittest/actions/return/err.not_allowed-1.d
> > new file mode 100644
> > index 00000000..246a68b1
> > --- /dev/null
> > +++ b/test/unittest/actions/return/err.not_allowed-1.d
> > @@ -0,0 +1,27 @@
> > +/*
> > + * Oracle Linux DTrace.
> > + * Copyright (c) 2025, 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: return() is not allowed for fbt probes
> > + *
> > + * SECTION: Actions and Subroutines/return()
> > + */
> > +
> > +#pragma D option quiet
> > +#pragma D option destructive
> > +
> > +BEGIN
> > +{
> > +	ok = 0;
> > +	exit(0);
> > +}
> > +
> > +fbt:btrfs:open_ctree:entry
> > +/ok/
> > +{
> > +	return(0);
> > +}
> > diff --git a/test/unittest/actions/return/err.not_allowed-1.r b/test/unittest/actions/return/err.not_allowed-1.r
> > new file mode 100644
> > index 00000000..e3443c30
> > --- /dev/null
> > +++ b/test/unittest/actions/return/err.not_allowed-1.r
> > @@ -0,0 +1,2 @@
> > +-- @@stderr --
> > +dtrace: could not enable tracing: return() not allowed for fbt:btrfs:open_ctree:entry
> > diff --git a/test/unittest/actions/return/err.not_allowed-2.d b/test/unittest/actions/return/err.not_allowed-2.d
> > new file mode 100644
> > index 00000000..e0d303c7
> > --- /dev/null
> > +++ b/test/unittest/actions/return/err.not_allowed-2.d
> > @@ -0,0 +1,27 @@
> > +/*
> > + * Oracle Linux DTrace.
> > + * Copyright (c) 2025, 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: return() is only allowed for select functions
> > + *
> > + * SECTION: Actions and Subroutines/return()
> > + */
> > +
> > +#pragma D option quiet
> > +#pragma D option destructive
> > +
> > +BEGIN
> > +{
> > +	ok = 0;
> > +	exit(0);
> > +}
> > +
> > +rawfbt:btrfs:close_ctree:entry
> > +/ok/
> > +{
> > +	return(0);
> > +}
> > diff --git a/test/unittest/actions/return/err.not_allowed-2.r b/test/unittest/actions/return/err.not_allowed-2.r
> > new file mode 100644
> > index 00000000..758d3477
> > --- /dev/null
> > +++ b/test/unittest/actions/return/err.not_allowed-2.r
> > @@ -0,0 +1,2 @@
> > +-- @@stderr --
> > +dtrace: could not enable tracing: return() not allowed for rawfbt:btrfs:close_ctree:entry
> > diff --git a/test/unittest/actions/return/err.not_allowed-3.d b/test/unittest/actions/return/err.not_allowed-3.d
> > new file mode 100644
> > index 00000000..770565c9
> > --- /dev/null
> > +++ b/test/unittest/actions/return/err.not_allowed-3.d
> > @@ -0,0 +1,27 @@
> > +/*
> > + * Oracle Linux DTrace.
> > + * Copyright (c) 2025, 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: return() is only allowed for select functions
> > + *
> > + * SECTION: Actions and Subroutines/return()
> > + */
> > +
> > +#pragma D option quiet
> > +#pragma D option destructive
> > +
> > +BEGIN
> > +{
> > +	ok = 0;
> > +	exit(0);
> > +}
> > +
> > +rawfbt:btrfs:*_ctree:entry
> > +/ok/
> > +{
> > +	return(0);
> > +}
> > diff --git a/test/unittest/actions/return/err.not_allowed-3.r b/test/unittest/actions/return/err.not_allowed-3.r
> > new file mode 100644
> > index 00000000..758d3477
> > --- /dev/null
> > +++ b/test/unittest/actions/return/err.not_allowed-3.r
> > @@ -0,0 +1,2 @@
> > +-- @@stderr --
> > +dtrace: could not enable tracing: return() not allowed for rawfbt:btrfs:close_ctree:entry

> From a6a6ceb614fadd5c5d1116cc05fe71da2b2dbff7 Mon Sep 17 00:00:00 2001
> From: Eugene Loh <eugene.loh@oracle.com>
> Date: Tue, 15 Jul 2025 13:33:04 -0700
> Subject: [PATCH] test
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  .../actions/return/tst.destructive-getpid.r   | 12 +++++
>  .../actions/return/tst.destructive-getpid.r.p | 10 ++++
>  .../actions/return/tst.destructive-getpid.sh  | 48 +++++++++++++++++++
>  3 files changed, 70 insertions(+)
>  create mode 100644 test/unittest/actions/return/tst.destructive-getpid.r
>  create mode 100755 test/unittest/actions/return/tst.destructive-getpid.r.p
>  create mode 100755 test/unittest/actions/return/tst.destructive-getpid.sh
> 
> diff --git a/test/unittest/actions/return/tst.destructive-getpid.r b/test/unittest/actions/return/tst.destructive-getpid.r
> new file mode 100644
> index 00000000..b6bdf211
> --- /dev/null
> +++ b/test/unittest/actions/return/tst.destructive-getpid.r
> @@ -0,0 +1,12 @@
> +pid is mypid
> +mypid
> +22222
> +mypid
> +mypid
> +55555
> +mypid
> +mypid
> +mypid
> +mypid
> +mypid
> +
> diff --git a/test/unittest/actions/return/tst.destructive-getpid.r.p b/test/unittest/actions/return/tst.destructive-getpid.r.p
> new file mode 100755
> index 00000000..d6593c46
> --- /dev/null
> +++ b/test/unittest/actions/return/tst.destructive-getpid.r.p
> @@ -0,0 +1,10 @@
> +#!/usr/bin/gawk -f
> +
> +BEGIN { mypid = -1 }
> +
> +/^pid is [1-9][0-9]*$/ { mypid = $3; }
> +
> +{
> +        if ( $NF == mypid ) $NF = "mypid";
> +        print;
> +}
> diff --git a/test/unittest/actions/return/tst.destructive-getpid.sh b/test/unittest/actions/return/tst.destructive-getpid.sh
> new file mode 100755
> index 00000000..7c44dde9
> --- /dev/null
> +++ b/test/unittest/actions/return/tst.destructive-getpid.sh
> @@ -0,0 +1,48 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2025, 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.
> +#
> +
> +dtrace=$1
> +
> +# Set up test directory.
> +
> +DIRNAME=$tmpdir/destructive-getpid.$$.$RANDOM
> +mkdir -p $DIRNAME
> +cd $DIRNAME
> +
> +# Make the trigger.  It reports the pid 10 times.
> +
> +cat << EOF > main.c
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +int main(int c, char **v) {
> +	int i;
> +
> +	for (i = 0; i < 10; i++)
> +		printf("%d\n", getpid());
> +
> +	return 0;
> +}
> +EOF
> +
> +$CC main.c
> +if [ $? -ne 0 ]; then
> +	echo ERROR: compile
> +	exit 1
> +fi
> +
> +# Trace the trigger.  On the 2nd and 5th getpid() calls, modify the result.
> +
> +$dtrace -c ./a.out -w -q -n '
> +BEGIN { printf("pid is %d\n", $target); n = 0; }
> +rawfbt:vmlinux:__*_sys_getpid:entry /pid == $target/ { n++ }
> +rawfbt:vmlinux:__*_sys_getpid:entry /pid == $target && n == 2/ { return(22222); }
> +rawfbt:vmlinux:__*_sys_getpid:entry /pid == $target && n == 5/ { return(55555); }
> +'
> +
> +exit $?
> -- 
> 2.18.4
> 

Re: [DTrace-devel] [PATCH 4/4] rawfbt: selectively allow return() in clauses

[Nick Alcock]

On 15 Jul 2025, Kris Van Hees uttered the following:

> On Tue, Jul 15, 2025 at 11:50:21AM +0100, Nick Alcock wrote:
>> On 15 Jul 2025, Kris Van Hees via DTrace-devel verbalised:
>> > +static int
>> > +fun_cmp(const allowed_fun_t *p, const allowed_fun_t *q) {
>> > +	int	rc;
>> > +
>> > +	if (p->mod != NULL) {
>> > +		if (q->mod == NULL)
>> > +			return 1;
>> > +		else {
>> > +			rc = strcmp(p->mod, q->mod);
>> > +			if (rc != 0)
>> > +				return rc;
>> > +		}
>> > +	} else if (q->mod != NULL)
>> > +		return -1;
>> 
>> Ugh, that's quite tangled for how little it does :( can't think of a
>> nicer approach though.
>
> I am hoping that perhaps in time a lot more functions may allow this, plus if
> someone is going to make use of this feature for error injection they are
> likely to use it for multiple functions and then being able to use a hashtable
> rather than scanning the list multiple times is nicer.

It's not the hashtable I was whining about, just the comparison
function. The hashtable absolutely makes sense.

-- 
NULL && (void)