Re: [PATCH v2] usb: typec: tps6598x: Remove VLA usage

[Heikki Krogerus]

On Mon, Jun 25, 2018 at 03:23:16PM -0700, Kees Cook wrote:
> In the quest to remove all stack VLA usage from the kernel[1], this
> uses the maximum buffer size and adds a sanity check. While 25 bytes
> is the size of the largest current things coming through, Heikki
> Krogerus pointed out that the actual max in 64 bytes, as per ch 1.3.2
> http://www.ti.com/lit/ug/slvuan1a/slvuan1a.pdf
> 
> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@xxxxxxxxxxxxxx
> 
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>

Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

> ---
> v2: use 64 bytes (Heikki)
> ---
>  drivers/usb/typec/tps6598x.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index 4b4c8d271b27..c84c8c189e90 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -81,12 +81,21 @@ struct tps6598x {
>  	struct typec_capability typec_cap;
>  };
>  
> +/*
> + * Max data bytes for Data1, Data2, and other registers. See ch 1.3.2:
> + * http://www.ti.com/lit/ug/slvuan1a/slvuan1a.pdf
> + */
> +#define TPS_MAX_LEN	64
> +
>  static int
>  tps6598x_block_read(struct tps6598x *tps, u8 reg, void *val, size_t len)
>  {
> -	u8 data[len + 1];
> +	u8 data[TPS_MAX_LEN + 1];
>  	int ret;
>  
> +	if (WARN_ON(len + 1 > sizeof(data)))
> +		return -EINVAL;
> +
>  	if (!tps->i2c_protocol)
>  		return regmap_raw_read(tps->regmap, reg, val, len);

Thanks,

-- 
heikki