[DTrace-devel] [PATCH 4/6] dtprobed: harden helper ioctl validation

[Alan Maguire]

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 at 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 at 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 at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
>