ACK: [PATCH] opal: add more bounds checking and zero pstates array

[Alex Hung]

On 2019-08-29 9:05 a.m., Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
> 
> Currently there is no bounds checking on the maximum number
> of pstates, so we can potentially have an out-of-bounds read
> on the pstates array. Fix this by checking and force setting
> maximum bounds.  Also zero the pstates array to ensure no
> uninitialized pstates values are printed.  Finally add some
> missing spaces in some warning messages.
> 
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>  src/opal/power_mgmt_info.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/opal/power_mgmt_info.c b/src/opal/power_mgmt_info.c
> index ab90b9f2..1175278c 100644
> --- a/src/opal/power_mgmt_info.c
> +++ b/src/opal/power_mgmt_info.c
> @@ -152,6 +152,7 @@ static int pstate_limits_test(fwts_framework *fw)
>  		return FWTS_ERROR;
>  	}
>  
> +	memset(pstates, 0, sizeof(pstates));
>  	ret = fwts_dt_property_read_u32_arr(fw->fdt, offset, "ibm,pstate-ids",
>  					pstates, &len);
>  	if (ret != FWTS_OK) {
> @@ -179,12 +180,12 @@ static int pstate_limits_test(fwts_framework *fw)
>  
>  	if (proc_gen == proc_gen_p8 && nr_pstates > 128)
>  		fwts_log_warning(fw,
> -				"More than 128 pstates found,nr_pstates = %d",
> +				"More than 128 pstates found, nr_pstates = %d",
>  				 nr_pstates);
>  
>  	if (proc_gen == proc_gen_p9 && nr_pstates > 255)
>  		fwts_log_warning(fw,
> -				"More than 255 pstates found,nr_pstates = %d",
> +				"More than 255 pstates found, nr_pstates = %d",
>  				 nr_pstates);
>  
>  	if (len != nr_pstates)
> @@ -192,6 +193,12 @@ static int pstate_limits_test(fwts_framework *fw)
>  				"Expected %d pstates, found %d pstates",
>  				nr_pstates, len);
>  
> +	/* Avoid reads outside of pstates[] */
> +	if (nr_pstates > MAX_PSTATES) {
> +		nr_pstates = MAX_PSTATES;
> +		fwts_log_warning(fw, "Truncating to %d pstates\n", nr_pstates);
> +	}
> +
>  	for (i = 0; i < nr_pstates; i++) {
>  		if (cmp_pstates(pstate_max, pstates[i]) < 0) {
>  			fwts_log_warning(fw, "Invalid Pstate id %d "
> 
Acked-by: Alex Hung <alex.hung at canonical.com>