Linux DTrace development list
 help / color / mirror / Atom feed
* [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
* [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
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