* [PATCH 4/6] dtprobed: harden helper ioctl validation
@ 2026-08-28 19:09 Kris Van Hees
2026-08-30 10:43 ` [DTrace-devel] " Alan Maguire
0 siblings, 1 reply; 3+ messages in thread
From: Kris Van Hees @ 2026-08-28 19:09 UTC (permalink / raw)
To: dtrace, dtrace-devel
Reject unterminated fixed-size module names and module names that would
be unsafe as pathname components before using helper data. Reuse the same
component-safety predicate used when composing probe specs, and handle
userdata allocation failure before dereferencing it.
Orabug: 39374493
Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
dtprobed/dof_stash.c | 27 +++++++++++++++------------
dtprobed/dof_stash.h | 2 ++
dtprobed/dtprobed.c | 22 ++++++++++++++++++++++
3 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/dtprobed/dof_stash.c b/dtprobed/dof_stash.c
index 4c7a95cd..891d7a69 100644
--- a/dtprobed/dof_stash.c
+++ b/dtprobed/dof_stash.c
@@ -230,6 +230,17 @@ make_provpid_name(const char *prov, pid_t pid)
return ret;
}
+/*
+ * Ban "." and ".." as probe description components. Components are also not
+ * allowed to contain any '/' character.
+ */
+int
+is_component_unsafe(const char *s)
+{
+ return strcmp(s, ".") == 0 || strcmp(s, "..") == 0 ||
+ strchr(s, '/') != NULL;
+}
+
/*
* Compose a full probespec's name from pieces.
*/
@@ -240,19 +251,11 @@ make_probespec_name(const char *prov, const char *mod, const char *fn,
char *ret;
/*
- * Ban "." and ".." as probe description components, as well as any
- * components with a '/' character. Since the components are used in
- * the creation of paths that will be written to, any of these cases
- * can be unsafe.
+ * Since probe description components are used in the creation of paths
+ * that will be written to, make sure their content is safe.
*/
- if (strcmp(prov, ".") == 0 || strcmp(prov, "..") == 0 ||
- strchr(prov, '/') != NULL ||
- strcmp(mod, ".") == 0 || strcmp(mod, "..") == 0 ||
- strchr(mod, '/') != NULL ||
- strcmp(fn, ".") == 0 || strcmp(fn, "..") == 0 ||
- strchr(fn, '/') != NULL ||
- strcmp(prb, ".") == 0 || strcmp(prb, "..") == 0 ||
- strchr(prb, '/') != NULL)
+ if (is_component_unsafe(prov) || is_component_unsafe(mod) ||
+ is_component_unsafe(fn) || is_component_unsafe(prb))
return NULL;
if (asprintf(&ret, "%s:%s:%s:%s", prov, mod, fn, prb) < 0) {
diff --git a/dtprobed/dof_stash.h b/dtprobed/dof_stash.h
index 32b5eb52..e72671af 100644
--- a/dtprobed/dof_stash.h
+++ b/dtprobed/dof_stash.h
@@ -21,6 +21,8 @@ typedef struct dof_parsed_list {
int dof_stash_init(const char *statedir);
+int is_component_unsafe(const char *s);
+
int dof_stash_push_parsed(dt_list_t *accum, dof_parsed_t *parsed);
int dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum);
void dof_stash_free(dt_list_t *accum);
diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
index 9ce941ee..c85a6617 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -668,6 +668,16 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
int gen;
usdt_data_t data;
+ /* If userdata == NULL, we were not able to allocate memory for it. */
+ if (userdata == NULL) {
+ fuse_log(FUSE_LOG_ERR, "%i: dtprobed: %s\n", pid,
+ "out of memory allocating userdata\n");
+ if (fuse_reply_err(req, ENOMEM) < 0)
+ fuse_log(FUSE_LOG_ERR, "%i: dtprobed: %s\n", pid,
+ "cannot send error to ioctl caller\n");
+ return;
+ }
+
/*
* We can just ignore FUSE_IOCTL_COMPAT: the 32-bit and 64-bit versions
* of the DOF structures are intentionally identical.
@@ -729,7 +739,19 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
errmsg, sizeof(dof_helper_t), in_bufsz);
goto fuse_err;
}
+
memcpy(&userdata->dh, in_buf, sizeof(dof_helper_t));
+ if (memchr(userdata->dh.dofhp_mod, 0, DTRACE_MODNAMELEN) == NULL) {
+ fuse_log(FUSE_LOG_ERR, "%i: dtprobed: "
+ "unterminated module name\n", pid);
+ goto fuse_err;
+ }
+ if (is_component_unsafe(userdata->dh.dofhp_mod)) {
+ fuse_log(FUSE_LOG_ERR, "%i: dtprobed: "
+ "unsafe characters in module name %s\n",
+ pid, userdata->dh.dofhp_mod);
+ goto fuse_err;
+ }
in.iov_base = (void *) userdata->dh.dofhp_dof;
in.iov_len = sizeof(dof_hdr_t);
--
2.52.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [DTrace-devel] [PATCH 4/6] dtprobed: harden helper ioctl validation
2026-08-28 19:09 [PATCH 4/6] dtprobed: harden helper ioctl validation Kris Van Hees
@ 2026-08-30 10:43 ` Alan Maguire
0 siblings, 0 replies; 3+ messages in thread
From: Alan Maguire @ 2026-08-30 10:43 UTC (permalink / raw)
To: Kris Van Hees, dtrace, dtrace-devel
On 28/08/2026 20:09, Kris Van Hees via DTrace-devel wrote:
> Reject unterminated fixed-size module names and module names that would
> be unsafe as pathname components before using helper data. Reuse the same
> component-safety predicate used when composing probe specs, and handle
> userdata allocation failure before dereferencing it.
>
> Orabug: 39374493
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
We may need to revisit for system-wide tracing for USDT (where module name
specifies a file path containing probes), but for now
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> dtprobed/dof_stash.c | 27 +++++++++++++++------------
> dtprobed/dof_stash.h | 2 ++
> dtprobed/dtprobed.c | 22 ++++++++++++++++++++++
> 3 files changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/dtprobed/dof_stash.c b/dtprobed/dof_stash.c
> index 4c7a95cd..891d7a69 100644
> --- a/dtprobed/dof_stash.c
> +++ b/dtprobed/dof_stash.c
> @@ -230,6 +230,17 @@ make_provpid_name(const char *prov, pid_t pid)
> return ret;
> }
>
> +/*
> + * Ban "." and ".." as probe description components. Components are also not
> + * allowed to contain any '/' character.
> + */
> +int
> +is_component_unsafe(const char *s)
> +{
> + return strcmp(s, ".") == 0 || strcmp(s, "..") == 0 ||
> + strchr(s, '/') != NULL;
> +}
> +
> /*
> * Compose a full probespec's name from pieces.
> */
> @@ -240,19 +251,11 @@ make_probespec_name(const char *prov, const char *mod, const char *fn,
> char *ret;
>
> /*
> - * Ban "." and ".." as probe description components, as well as any
> - * components with a '/' character. Since the components are used in
> - * the creation of paths that will be written to, any of these cases
> - * can be unsafe.
> + * Since probe description components are used in the creation of paths
> + * that will be written to, make sure their content is safe.
> */
> - if (strcmp(prov, ".") == 0 || strcmp(prov, "..") == 0 ||
> - strchr(prov, '/') != NULL ||
> - strcmp(mod, ".") == 0 || strcmp(mod, "..") == 0 ||
> - strchr(mod, '/') != NULL ||
> - strcmp(fn, ".") == 0 || strcmp(fn, "..") == 0 ||
> - strchr(fn, '/') != NULL ||
> - strcmp(prb, ".") == 0 || strcmp(prb, "..") == 0 ||
> - strchr(prb, '/') != NULL)
> + if (is_component_unsafe(prov) || is_component_unsafe(mod) ||
> + is_component_unsafe(fn) || is_component_unsafe(prb))
> return NULL;
>
> if (asprintf(&ret, "%s:%s:%s:%s", prov, mod, fn, prb) < 0) {
> diff --git a/dtprobed/dof_stash.h b/dtprobed/dof_stash.h
> index 32b5eb52..e72671af 100644
> --- a/dtprobed/dof_stash.h
> +++ b/dtprobed/dof_stash.h
> @@ -21,6 +21,8 @@ typedef struct dof_parsed_list {
>
> int dof_stash_init(const char *statedir);
>
> +int is_component_unsafe(const char *s);
> +
> int dof_stash_push_parsed(dt_list_t *accum, dof_parsed_t *parsed);
> int dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum);
> void dof_stash_free(dt_list_t *accum);
> diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
> index 9ce941ee..c85a6617 100644
> --- a/dtprobed/dtprobed.c
> +++ b/dtprobed/dtprobed.c
> @@ -668,6 +668,16 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
> int gen;
> usdt_data_t data;
>
> + /* If userdata == NULL, we were not able to allocate memory for it. */
> + if (userdata == NULL) {
> + fuse_log(FUSE_LOG_ERR, "%i: dtprobed: %s\n", pid,
> + "out of memory allocating userdata\n");
> + if (fuse_reply_err(req, ENOMEM) < 0)
> + fuse_log(FUSE_LOG_ERR, "%i: dtprobed: %s\n", pid,
> + "cannot send error to ioctl caller\n");
> + return;
> + }
> +
> /*
> * We can just ignore FUSE_IOCTL_COMPAT: the 32-bit and 64-bit versions
> * of the DOF structures are intentionally identical.
> @@ -729,7 +739,19 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
> errmsg, sizeof(dof_helper_t), in_bufsz);
> goto fuse_err;
> }
> +
> memcpy(&userdata->dh, in_buf, sizeof(dof_helper_t));
> + if (memchr(userdata->dh.dofhp_mod, 0, DTRACE_MODNAMELEN) == NULL) {
> + fuse_log(FUSE_LOG_ERR, "%i: dtprobed: "
> + "unterminated module name\n", pid);
> + goto fuse_err;
> + }
> + if (is_component_unsafe(userdata->dh.dofhp_mod)) {
> + fuse_log(FUSE_LOG_ERR, "%i: dtprobed: "
> + "unsafe characters in module name %s\n",
> + pid, userdata->dh.dofhp_mod);
> + goto fuse_err;
> + }
>
> in.iov_base = (void *) userdata->dh.dofhp_dof;
> in.iov_len = sizeof(dof_hdr_t);
> --
> 2.52.0
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 4/6] dtprobed: harden helper ioctl validation
@ 2026-06-12 16:22 Kris Van Hees
0 siblings, 0 replies; 3+ messages in thread
From: Kris Van Hees @ 2026-06-12 16:22 UTC (permalink / raw)
To: dtrace, dtrace-devel
Reject unterminated fixed-size module names and module names that would
be unsafe as pathname components before using helper data. Reuse the same
component-safety predicate used when composing probe specs, and handle
userdata allocation failure before dereferencing it.
Orabug: 39374493
Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
dtprobed/dof_stash.c | 27 +++++++++++++++------------
dtprobed/dof_stash.h | 2 ++
dtprobed/dtprobed.c | 22 ++++++++++++++++++++++
3 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/dtprobed/dof_stash.c b/dtprobed/dof_stash.c
index 4c7a95cd..891d7a69 100644
--- a/dtprobed/dof_stash.c
+++ b/dtprobed/dof_stash.c
@@ -230,6 +230,17 @@ make_provpid_name(const char *prov, pid_t pid)
return ret;
}
+/*
+ * Ban "." and ".." as probe description components. Components are also not
+ * allowed to contain any '/' character.
+ */
+int
+is_component_unsafe(const char *s)
+{
+ return strcmp(s, ".") == 0 || strcmp(s, "..") == 0 ||
+ strchr(s, '/') != NULL;
+}
+
/*
* Compose a full probespec's name from pieces.
*/
@@ -240,19 +251,11 @@ make_probespec_name(const char *prov, const char *mod, const char *fn,
char *ret;
/*
- * Ban "." and ".." as probe description components, as well as any
- * components with a '/' character. Since the components are used in
- * the creation of paths that will be written to, any of these cases
- * can be unsafe.
+ * Since probe description components are used in the creation of paths
+ * that will be written to, make sure their content is safe.
*/
- if (strcmp(prov, ".") == 0 || strcmp(prov, "..") == 0 ||
- strchr(prov, '/') != NULL ||
- strcmp(mod, ".") == 0 || strcmp(mod, "..") == 0 ||
- strchr(mod, '/') != NULL ||
- strcmp(fn, ".") == 0 || strcmp(fn, "..") == 0 ||
- strchr(fn, '/') != NULL ||
- strcmp(prb, ".") == 0 || strcmp(prb, "..") == 0 ||
- strchr(prb, '/') != NULL)
+ if (is_component_unsafe(prov) || is_component_unsafe(mod) ||
+ is_component_unsafe(fn) || is_component_unsafe(prb))
return NULL;
if (asprintf(&ret, "%s:%s:%s:%s", prov, mod, fn, prb) < 0) {
diff --git a/dtprobed/dof_stash.h b/dtprobed/dof_stash.h
index 32b5eb52..e72671af 100644
--- a/dtprobed/dof_stash.h
+++ b/dtprobed/dof_stash.h
@@ -21,6 +21,8 @@ typedef struct dof_parsed_list {
int dof_stash_init(const char *statedir);
+int is_component_unsafe(const char *s);
+
int dof_stash_push_parsed(dt_list_t *accum, dof_parsed_t *parsed);
int dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum);
void dof_stash_free(dt_list_t *accum);
diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
index 9ce941ee..c85a6617 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -668,6 +668,16 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
int gen;
usdt_data_t data;
+ /* If userdata == NULL, we were not able to allocate memory for it. */
+ if (userdata == NULL) {
+ fuse_log(FUSE_LOG_ERR, "%i: dtprobed: %s\n", pid,
+ "out of memory allocating userdata\n");
+ if (fuse_reply_err(req, ENOMEM) < 0)
+ fuse_log(FUSE_LOG_ERR, "%i: dtprobed: %s\n", pid,
+ "cannot send error to ioctl caller\n");
+ return;
+ }
+
/*
* We can just ignore FUSE_IOCTL_COMPAT: the 32-bit and 64-bit versions
* of the DOF structures are intentionally identical.
@@ -729,7 +739,19 @@ helper_ioctl(fuse_req_t req, int cmd, void *arg,
errmsg, sizeof(dof_helper_t), in_bufsz);
goto fuse_err;
}
+
memcpy(&userdata->dh, in_buf, sizeof(dof_helper_t));
+ if (memchr(userdata->dh.dofhp_mod, 0, DTRACE_MODNAMELEN) == NULL) {
+ fuse_log(FUSE_LOG_ERR, "%i: dtprobed: "
+ "unterminated module name\n", pid);
+ goto fuse_err;
+ }
+ if (is_component_unsafe(userdata->dh.dofhp_mod)) {
+ fuse_log(FUSE_LOG_ERR, "%i: dtprobed: "
+ "unsafe characters in module name %s\n",
+ pid, userdata->dh.dofhp_mod);
+ goto fuse_err;
+ }
in.iov_base = (void *) userdata->dh.dofhp_dof;
in.iov_len = sizeof(dof_hdr_t);
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-30 10:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-28 19:09 [PATCH 4/6] dtprobed: harden helper ioctl validation Kris Van Hees
2026-08-30 10:43 ` [DTrace-devel] " Alan Maguire
-- strict thread matches above, loose matches on Subject: below --
2026-06-12 16:22 Kris Van Hees
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox