[DTrace-devel] [PATCH v3 05/19] alloca: new bad-size fault

[Kris Van Hees]

On Thu, Mar 24, 2022 at 06:24:31PM +0000, Nick Alcock via DTrace-devel wrote:
> We'll use DTRACEFLT_BADSIZE to distinguish in error messages between
> out-of-bounds errors in bcopy and the like, and size-too-large errors.
> (Without this, it's hard to tell if a, say, 0x12 in the fault value
> is a nearly-NULL pointer or a wrong size.)

Looking at the patch series, I only see this used on patch 10/19, in the
implementation of dt_cg_check_outscratch_bounds() which got added because
of the bcopy implementation needing it.  I don't think it is really conveying
information that is relevant to a user.  If the size is deemed out of range,
then you still face two potential causes:

  (1) the base address is valid but we are trying to access more data than we
      should
  (2) the base address is invalid and we are actually trying to access a valid
      mount of data

I don't think you can really distinguish between those two cases, so even with
adding a BADSIZE fault, you might be reporting that one when the real cause is
actually a BADADDR.

As such, I think we should just report BADADDR and leave it at that  invalid and we are actually trying to access a valid
      mount of data

I don't think you can really distinguish between those two cases, so even with
adding a BADSIZE fault, you might be reporting that one when the real cause is
actually a BADADDR.

As such, I think we should just report BADADDR and leave it at that.  That is
consistent behaviour and not unreasonable because the end result of trying to
access SIZE data at ADDR would logically be a BADADDR (trying to access an
address that we shouldn't), regardless of whether SIZE or ADDR is invalid.

> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
>  include/dtrace/faults_defines.h | 1 +
>  libdtrace/dt_error.c            | 1 +
>  libdtrace/dt_handle.c           | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/include/dtrace/faults_defines.h b/include/dtrace/faults_defines.h
> index 20c57ad77499..3bef92d251ca 100644
> --- a/include/dtrace/faults_defines.h
> +++ b/include/dtrace/faults_defines.h
> @@ -32,6 +32,7 @@
>  #define DTRACEFLT_UPRIV			7	/* Illegal user access */
>  #define DTRACEFLT_TUPOFLOW		8	/* Tuple stack overflow */
>  #define DTRACEFLT_BADSTACK		9	/* Bad stack */
> +#define DTRACEFLT_BADSIZE		10	/* Bad size */
>  
>  #define DTRACEFLT_LIBRARY		1000	/* Library-level fault */
>  
> diff --git a/libdtrace/dt_error.c b/libdtrace/dt_error.c
> index 50229d271425..13713acb5f23 100644
> --- a/libdtrace/dt_error.c
> +++ b/libdtrace/dt_error.c
> @@ -206,6 +206,7 @@ dtrace_faultstr(dtrace_hdl_t *dtp, int fault)
>  		{ DTRACEFLT_UPRIV,	"invalid user access" },
>  		{ DTRACEFLT_TUPOFLOW,	"tuple stack overflow" },
>  		{ DTRACEFLT_BADSTACK,	"bad stack" },
> +		{ DTRACEFLT_BADSIZE,	"invalid size" },
>  		{ DTRACEFLT_LIBRARY,	"library-level fault" },
>  		{ 0,			NULL }
>  	};
> diff --git a/libdtrace/dt_handle.c b/libdtrace/dt_handle.c
> index 433ce4519cb3..90fea09182db 100644
> --- a/libdtrace/dt_handle.c
> +++ b/libdtrace/dt_handle.c
> @@ -181,6 +181,7 @@ dt_handle_err(dtrace_hdl_t *dtp, dtrace_probedata_t *data)
>  	case DTRACEFLT_BADADDR:
>  	case DTRACEFLT_BADALIGN:
>  	case DTRACEFLT_BADSTACK:
> +	case DTRACEFLT_BADSIZE:
>  		sprintf(details, " (0x%llx)", (u_longlong_t)err.dteda_addr);
>  		break;
>  
> -- 
> 2.35.1.261.g8402f930ba.dirty
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel