[DTrace-devel] [PATCH v2] parser: do not bounds-check arrays of size 0 or 1

[Kris Van Hees]

On Tue, Sep 12, 2023 at 08:17:11PM +0100, Nick Alcock wrote:
> This is both a pretty useless degenerate check to carry out, and breaks when
> -fstrict-flex-arrays is used (whereupon trailing old-style flexible arrays
> gain bounds of 0).  It is almost certainly wrong to use trailing zero-size
> arrays and -fstrict-flex-arrays in conjunction, but since C doesn't
> bounds-check this is commonplace.  In particular the Linux kernel has turned
> this on *before* transitioning away from such arrays, rather than
> afterwards.
> 
> Fixes system translators (in particular the pr_pgid and pr_sid members of
> psinfo_t) in conjunction with Linux 6.5+.
> 
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

> ---
>  libdtrace/dt_parser.c                         | 24 ++++++++++---------
>  .../unittest/arrays/tst.ctf-dynsized-bounds.d | 20 ++++++++++++++++
>  2 files changed, 33 insertions(+), 11 deletions(-)
>  create mode 100644 test/unittest/arrays/tst.ctf-dynsized-bounds.d
> 
> diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
> index beb94676cd10c..f6addc78e32ba 100644
> --- a/libdtrace/dt_parser.c
> +++ b/libdtrace/dt_parser.c
> @@ -3580,23 +3580,25 @@ dt_cook_op2(dt_node_t *dnp, uint_t idflags)
>  
>  		/*
>  		 * Array bounds-checking.  (Non-associative arrays only.)
> +		 *
> +		 * Checking for arrays of size 0 and 1 is skipped: these
> +		 * degenerate cases are often used for dynamically-sized arrays
> +		 * at the ends of structures.
>  		 */
>  		artype = ctf_type_resolve(lp->dn_ctfp, lp->dn_type);
>  		arkind = ctf_type_kind(lp->dn_ctfp, artype);
>  
>  		if (arkind == CTF_K_ARRAY &&
>  		    !(lp->dn_kind == DT_NODE_VAR &&
> -			lp->dn_ident->di_kind == DT_IDENT_ARRAY)) {
> -			ctf_array_info(lp->dn_ctfp, artype, &r);
> -
> -			if (rp->dn_kind == DT_NODE_INT &&
> -			    ctf_array_info(lp->dn_ctfp, type, &r) == 0 &&
> -			    rp->dn_value >= r.ctr_nelems)
> -				xyerror(D_ARR_BOUNDS, "index outside "
> -				    "array bounds: %llu, max is %i\n",
> -				    (long long unsigned)rp->dn_value,
> -				    r.ctr_nelems);
> -		}
> +			lp->dn_ident->di_kind == DT_IDENT_ARRAY) &&
> +		    rp->dn_kind == DT_NODE_INT &&
> +		    ctf_array_info(lp->dn_ctfp, type, &r) == 0 &&
> +		    r.ctr_nelems > 1 &&
> +		    rp->dn_value >= r.ctr_nelems)
> +			xyerror(D_ARR_BOUNDS, "index outside "
> +				"array bounds: %llu, max is %i\n",
> +				(long long unsigned)rp->dn_value,
> +				r.ctr_nelems);
>  
>  		dt_node_type_assign(dnp, ctfp, type);
>  		dt_node_attr_assign(dnp, dt_attr_min(lp->dn_attr, rp->dn_attr));
> diff --git a/test/unittest/arrays/tst.ctf-dynsized-bounds.d b/test/unittest/arrays/tst.ctf-dynsized-bounds.d
> new file mode 100644
> index 0000000000000..9479a1e14c9b6
> --- /dev/null
> +++ b/test/unittest/arrays/tst.ctf-dynsized-bounds.d
> @@ -0,0 +1,20 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2018, 2022, 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.
> + */
> +
> +/*
> + * ASSERTION:
> + * 	Array accesses work for CTF-declared arrays of dynamic size.
> + * 	pr_pgid is one such.
> + *
> + * SECTION: Pointers and Arrays/Array Declarations and Storage
> + */
> +
> +BEGIN
> +{
> +	trace(curpsinfo->pr_pgid);
> +	exit(0);
> +}
> -- 
> 2.42.0.271.g85384428f1