[DTrace-devel] [PATCH] Fix double free of printf descriptors

[Kris Van Hees]

Hm, clearly I need to push my development tree patches to 2.0-branch-dev
faster, since this is what I address in:

3c4bb9b8 Fix double-free on printf format structure

on kvh/2.0-branch-dev

Sorry about that - let me push the latest to 2.0-branch-dev, since that will
give a clearer picture of stuff.  I was holding off because the series for
the compiler changes should really go together at this point, but I forgot I
had a few extra patches sitting in that tree that really need to make it out
(and are not directly related to the compiler changes - only happened to be
found while working on that code).

On Mon, Jul 13, 2020 at 05:09:28PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> A recent commit ("Ensure record descriptors get cleaned up") sought
> to free printf descriptors associated with any record descriptors.
> It did so by having dt_datadesc_release() loop over records, freeing
> a printf descriptor (if any) for each record.
> 
> The problem was that there should be a match between dt_printf_create()
> and dt_printf_destroy() calls, but in dt_cg_act_printf() the same
> printf descriptor can be used for multiple records.  The patch would
> try to free the same printf descriptor multiple times.
> 
> For example,
> 
>     # dtrace -qn 'BEGIN { printf("%d %d %d\n", 1, 2, 3); exit(0); }'
>     1 2 3
>     double free or corruption (fasttop)
>     # ./runtest.sh test/unittest/actions/printf/tst.conv_T.sh \
>                    test/unittest/actions/printf/tst.conv_Y.sh \
>                    test/unittest/builtinvar/tst.arg1to8.d \
>                    test/unittest/inline/tst.InlineExpression.d \
>                    test/unittest/types/tst.bitops.d \
>     test/unittest/actions/printf/tst.conv_T.sh: FAIL: core dumped.
>     test/unittest/actions/printf/tst.conv_Y.sh: FAIL: core dumped.
>     test/unittest/builtinvar/tst.arg1to8.d: FAIL: core dumped.
>     test/unittest/inline/tst.InlineExpression.d: FAIL: core dumped.
>     test/unittest/types/tst.bitops.d: FAIL: core dumped.
>     5 cases (0 PASS, 5 FAIL, 0 XPASS, 0 XFAIL, 0 SKIP)
> 
> There are also other such test failures, but they're hiding behind
> XFAILs.
> 
> Fix dt_datadesc_release() to free a particular printf descriptor
> only once.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_map.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libdtrace/dt_map.c b/libdtrace/dt_map.c
> index 3994ec3a..43ac1f84 100644
> --- a/libdtrace/dt_map.c
> +++ b/libdtrace/dt_map.c
> @@ -26,13 +26,16 @@ dt_datadesc_release(dtrace_hdl_t *dtp, dtrace_datadesc_t *ddp)
>  {
>  	int			i;
>  	dtrace_recdesc_t	*rec;
> +	void			*last = NULL;
>  
>  	if (--ddp->dtdd_refcnt > 0)
>  		return;
>  
>  	for (i = 0, rec = &ddp->dtdd_recs[0]; i < ddp->dtdd_nrecs; i++, rec++) {
> -		if (rec->dtrd_format != NULL)
> +		if (rec->dtrd_format != NULL &&
> +		    rec->dtrd_format != last)
>  			dt_printf_destroy(rec->dtrd_format);
> +		last = rec->dtrd_format;
>  	}
>  	dt_free(dtp, ddp->dtdd_recs);
>  	dt_free(dtp, ddp);
> -- 
> 2.18.2
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel