Re: [PATCH] usb: isp1760: Fix out-of-bounds array access

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Linus,
Thanks for the patch.

On Mon, May 16, 2022 at 11:14:24AM +0200, Linus Walleij wrote:
> Running the driver through kasan gives an interesting splat:
> 
>   BUG: KASAN: global-out-of-bounds in isp1760_register+0x180/0x70c
>   Read of size 20 at addr f1db2e64 by task swapper/0/1
>   (...)
>   isp1760_register from isp1760_plat_probe+0x1d8/0x220
>   (...)
> 
> This happens because the loop reading the regmap fields for the
> different ISP1760 variants look like this:
> 
>   for (i = 0; i < HC_FIELD_MAX; i++) { ... }
> 
> Meaning it expects the arrays to be at least HC_FIELD_MAX - 1 long.
> 
> However the arrays isp1760_hc_reg_fields[], isp1763_hc_reg_fields[],
> isp1763_hc_volatile_ranges[] and isp1763_dc_volatile_ranges[] are
> dynamically sized during compilation.
> 
> Fix this by putting an empty assignment to the [HC_FIELD_MAX]
> and [DC_FIELD_MAX] array member at the end of each array.
> This will make the array one member longer than it needs to be,
> but avoids the risk of overwriting whatever is inside
> [HC_FIELD_MAX - 1] and is simple and intuitive to read. Also
> add comments explaining what is going on.
> 
> Fixes: 1da9e1c06873 ("usb: isp1760: move to regmap for register access")
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Rui Miguel Silva <rui.silva@xxxxxxxxxx>

Very good catch. Thanks for fixing this.

Reviewed-by: Rui Miguel Silva <rui.silva@xxxxxxxxxx>

Cheers,
   Rui

> Cc: Dietmar Eggemann <dietmar.eggemann@xxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> Found while testing to compile the Vexpress kernel into the
> vmalloc area in some experimental patches, curiously it did not
> manifest before, I guess we were lucky with padding
> etc.
> ---
>  drivers/usb/isp1760/isp1760-core.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/usb/isp1760/isp1760-core.c b/drivers/usb/isp1760/isp1760-core.c
> index d1d9a7d5da17..af88f4fe00d2 100644
> --- a/drivers/usb/isp1760/isp1760-core.c
> +++ b/drivers/usb/isp1760/isp1760-core.c
> @@ -251,6 +251,8 @@ static const struct reg_field isp1760_hc_reg_fields[] = {
>  	[HW_DM_PULLDOWN]	= REG_FIELD(ISP176x_HC_OTG_CTRL, 2, 2),
>  	[HW_DP_PULLDOWN]	= REG_FIELD(ISP176x_HC_OTG_CTRL, 1, 1),
>  	[HW_DP_PULLUP]		= REG_FIELD(ISP176x_HC_OTG_CTRL, 0, 0),
> +	/* Make sure the array is sized properly during compilation */
> +	[HC_FIELD_MAX]		= {},
>  };
>  
>  static const struct reg_field isp1763_hc_reg_fields[] = {
> @@ -321,6 +323,8 @@ static const struct reg_field isp1763_hc_reg_fields[] = {
>  	[HW_DM_PULLDOWN_CLEAR]	= REG_FIELD(ISP1763_HC_OTG_CTRL_CLEAR, 2, 2),
>  	[HW_DP_PULLDOWN_CLEAR]	= REG_FIELD(ISP1763_HC_OTG_CTRL_CLEAR, 1, 1),
>  	[HW_DP_PULLUP_CLEAR]	= REG_FIELD(ISP1763_HC_OTG_CTRL_CLEAR, 0, 0),
> +	/* Make sure the array is sized properly during compilation */
> +	[HC_FIELD_MAX]		= {},
>  };
>  
>  static const struct regmap_range isp1763_hc_volatile_ranges[] = {
> @@ -405,6 +409,8 @@ static const struct reg_field isp1761_dc_reg_fields[] = {
>  	[DC_CHIP_ID_HIGH]	= REG_FIELD(ISP176x_DC_CHIPID, 16, 31),
>  	[DC_CHIP_ID_LOW]	= REG_FIELD(ISP176x_DC_CHIPID, 0, 15),
>  	[DC_SCRATCH]		= REG_FIELD(ISP176x_DC_SCRATCH, 0, 15),
> +	/* Make sure the array is sized properly during compilation */
> +	[DC_FIELD_MAX]		= {},
>  };
>  
>  static const struct regmap_range isp1763_dc_volatile_ranges[] = {
> @@ -458,6 +464,8 @@ static const struct reg_field isp1763_dc_reg_fields[] = {
>  	[DC_CHIP_ID_HIGH]	= REG_FIELD(ISP1763_DC_CHIPID_HIGH, 0, 15),
>  	[DC_CHIP_ID_LOW]	= REG_FIELD(ISP1763_DC_CHIPID_LOW, 0, 15),
>  	[DC_SCRATCH]		= REG_FIELD(ISP1763_DC_SCRATCH, 0, 15),
> +	/* Make sure the array is sized properly during compilation */
> +	[DC_FIELD_MAX]		= {},
>  };
>  
>  static const struct regmap_config isp1763_dc_regmap_conf = {
> -- 
> 2.35.1
> 



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux