Re: [PATCH] Fix various memory leaks related to stapsdt and usdt probes

Reviewed-by: Eugene Loh <[email protected]>
(quick review)

On 8/6/25 09:58, Kris Van Hees wrote:
> Signed-off-by: Kris Van Hees <[email protected]>
> ---
>   libdtrace/dt_pid.c         | 14 ++++++++++----
>   libdtrace/dt_prov_uprobe.c | 13 +++++++++++++
>   2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
> index 42f667fe1..4af9141aa 100644
> --- a/libdtrace/dt_pid.c
> +++ b/libdtrace/dt_pid.c
> @@ -1282,6 +1282,7 @@ dt_stapsdt_parse(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t *pdp,
>   	int i, err = 0;
>   	int fd = -1;
>   	char *mod;
> +	char *no_fun = "";
>   
>   	fd = open(path, O_RDONLY);
>   	if (fd < 0) {
> @@ -1415,7 +1416,7 @@ dt_stapsdt_parse(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t *pdp,
>   				       &fun, &sym) == 0)
>   			psp.pps_fun = (char *)fun;
>   		else
> -			psp.pps_fun = "";
> +			psp.pps_fun = no_fun;
>   		psp.pps_dev = pmp->pr_dev;
>   		psp.pps_inum = pmp->pr_inum;
>   		psp.pps_pid = dpr->dpr_pid;
> @@ -1430,6 +1431,9 @@ dt_stapsdt_parse(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t *pdp,
>   		}
>   		if (err == -1)
>   			break;
> +
> +		if (psp.pps_fun != no_fun)
> +			free(psp.pps_fun);
>   	}
>   
>   out:
> @@ -1513,7 +1517,6 @@ dt_pid_create_stapsdt_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_
>   	const dt_provider_t *pvp;
>   	dt_proc_t *dpr = NULL;
>   	const char *pidstr;
> -	char *path = NULL;
>   	pid_t pid;
>   
>   	assert(pcb != NULL);
> @@ -1525,8 +1528,6 @@ dt_pid_create_stapsdt_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_
>   	if (strlen(pidstr) == 0)
>   		return 0;
>   
> -	asprintf(&path, "/proc/%s/maps", pidstr);
> -
>   	pvp = dt_provider_lookup(dtp, "stapsdt");
>   	assert(pvp != NULL);
>   
> @@ -1542,8 +1543,13 @@ dt_pid_create_stapsdt_probes(dtrace_probedesc_t *pdp, dtrace_hdl_t *dtp, dt_pcb_
>   	}
>   	dpr = dt_proc_lookup(dtp, pid);
>   	if (dpr) {
> +		char *path = NULL;
> +
> +		if (asprintf(&path, "/proc/%s/maps", pidstr) == -1)
> +			longjmp(pcb->pcb_jmpbuf, EDT_NOMEM);
>   		dt_pid_create_stapsdt_probes_proc(pdp, dtp, pcb,
>   						  pvp, dpr, path);
> +		free(path);
>   		dt_proc_release_unlock(dtp, pid);
>   	}
>   
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index 8c8dcae0c..986ac0ead 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -795,6 +795,10 @@ static int populate_args(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
>   	char	*nptr = NULL, *xptr = NULL;
>   	size_t	i;
>   
> +	/* Nothing to do if we already populated the arguments. */
> +	if (upp->argc >= 0)
> +		return 0;
> +
>   	upp->argc = psp->pps_xargc;
>   
>   	/* Copy argument value source string data (if any). */
> @@ -941,6 +945,7 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
>   		upp->refcntr_off = psp->pps_refcntr_off;
>   		upp->fn = strdup(psp->pps_fn);
>   		upp->func = NULL;
> +		upp->argc = -1;			/* no argument data yet */
>   		upp->tp = dt_tp_alloc(dtp);
>   		if (upp->tp == NULL)
>   			goto fail;
> @@ -1716,6 +1721,7 @@ static char *uprobe_create(dev_t dev, ino_t ino, const char *mapping_fn,
>   	rc = dprintf(fd, "%c:%s %s\n", flags & PP_IS_RETURN ? 'r' : 'p', name, spec);
>   
>   out:
> +	free(spec);
>   	if (fd != -1)
>   		close(fd);
>   	if (rc < 0) {
> @@ -1975,6 +1981,12 @@ static void detach(dtrace_hdl_t *dtp, const dt_probe_t *uprp)
>   	uprobe_delete(upp->dev, upp->inum, upp->off, upp->flags);
>   }
>   
> +/* Clean up the private provider data. */
> +static void destroy(dtrace_hdl_t *dtp, void *arg)
> +{
> +	dt_htab_destroy((dt_htab_t *)arg);
> +}
> +
>   /*
>    * Used for underlying probes (uprobes).
>    */
> @@ -2014,6 +2026,7 @@ dt_provimpl_t	dt_usdt = {
>   	.probe_destroy	= &probe_destroy,
>   	.discover	= &discover,
>   	.add_probe	= &add_probe_usdt,
> +	.destroy	= &destroy,
>   };
>   
>   /*