[DTrace-devel] [PATCH v2] Ensure string constants that are too long are truncated correctly

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>

This patch seems to orphan dt_strtab.  Specifically (not that this is a 
big deal), the zalloc() no longer has a partner free in dtrace_close() 
(or dt_difo_free... which I do not understand).

The two-byte encoding of the string-length prefix is hard-wired in lots 
of places but oh well.  Not a great practice, but so be it.

On 11/20/21 12:28 AM, Kris Van Hees via DTrace-devel wrote:
> The string table can contain strings that are longer than the maximum
> string size for the program being compiled.  They need to be truncated
> before they are used to ensure that string operations work correctly.
>
> Since string constants can be used verbatim or as values of built-in
> variables, it is easiest to just pre-process the string table before it
> is loaded into the 'strtab' BPF map.  Every string that is longer than
> the maximum string size has its length prefix set to the maximum string
> size and the string itself is truncated to that length.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_bpf.c                           | 27 ++++++++++++++++----
>   test/unittest/codegen/tst.str_const_length.d | 21 +++++++++++++++
>   test/unittest/codegen/tst.str_const_length.r |  4 +++
>   3 files changed, 47 insertions(+), 5 deletions(-)
>   create mode 100644 test/unittest/codegen/tst.str_const_length.d
>   create mode 100644 test/unittest/codegen/tst.str_const_length.r
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index e0134a93..fc1e5e8f 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -240,6 +240,9 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
>   	int		stabsz, gvarsz, lvarsz, tvarc, aggsz, memsz;
>   	int		ci_mapfd, st_mapfd, pr_mapfd;
>   	uint32_t	key = 0;
> +	size_t		strsize = dtp->dt_options[DTRACEOPT_STRSIZE];
> +	uint8_t		*buf, *end;
> +	char		*strtab;
>   
>   	/* If we already created the global maps, return success. */
>   	if (dt_gmap_done)
> @@ -325,13 +328,27 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
>   	 * value that is used to store the strtab.
>   	 */
>   	dtp->dt_strlen = dt_strtab_size(dtp->dt_ccstab);
> -	stabsz = dtp->dt_strlen + dtp->dt_options[DTRACEOPT_STRSIZE];
> -	dtp->dt_strtab = dt_zalloc(dtp, stabsz);
> -	if (dtp->dt_strtab == NULL)
> +	stabsz = dtp->dt_strlen + strsize;
> +	strtab = dt_zalloc(dtp, stabsz);
> +	if (strtab == NULL)
>   		return dt_set_errno(dtp, EDT_NOMEM);
>   
>   	dt_strtab_write(dtp->dt_ccstab, (dt_strtab_write_f *)dt_strtab_copystr,
> -			dtp->dt_strtab);
> +			strtab);
> +
> +	/* Loop over the string table and truncate strings that are too long. */
> +	buf = (uint8_t *)strtab;
> +	end = buf + dtp->dt_strlen;
> +	while (buf < end) {
> +		uint_t	len = (buf[0] << 8) | buf[1];
> +
> +		if (len > strsize) {
> +			buf[0] = strsize >> 8;
> +			buf[1] = strsize & 0xff;
> +			buf[2 + strsize] = '\0';
> +		}
> +		buf += 2 + len + 1;
> +	}
>   
>   	st_mapfd = create_gmap(dtp, "strtab", BPF_MAP_TYPE_ARRAY,
>   			       sizeof(uint32_t), stabsz, 1);
> @@ -363,7 +380,7 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
>   	dt_bpf_map_update(ci_mapfd, &key, dtp->dt_conf.cpus);
>   
>   	/* Populate the 'strtab' map. */
> -	dt_bpf_map_update(st_mapfd, &key, dtp->dt_strtab);
> +	dt_bpf_map_update(st_mapfd, &key, strtab);
>   
>   	/* Populate the 'probes' map. */
>   	populate_probes_map(dtp, pr_mapfd);
> diff --git a/test/unittest/codegen/tst.str_const_length.d b/test/unittest/codegen/tst.str_const_length.d
> new file mode 100644
> index 00000000..1c58ba13
> --- /dev/null
> +++ b/test/unittest/codegen/tst.str_const_length.d
> @@ -0,0 +1,21 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +#pragma D option rawbytes
> +#pragma D option strsize=5
> +#pragma D option quiet
> +
> +BEGIN
> +{
> +	trace("abcdefgh");
> +	exit(0);
> +}
> +
> +ERROR
> +{
> +	exit(1);
> +}
> diff --git a/test/unittest/codegen/tst.str_const_length.r b/test/unittest/codegen/tst.str_const_length.r
> new file mode 100644
> index 00000000..b6991a71
> --- /dev/null
> +++ b/test/unittest/codegen/tst.str_const_length.r
> @@ -0,0 +1,4 @@
> +
> +             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f  0123456789abcdef
> +         0: 00 05 61 62 63 64 65 00                          ..abcde.
> +