Message-ID: <8757b9bc-c771-a612-1d6a-1a704e7a7688@linaro.org>
Date:   Thu, 2 Feb 2023 21:38:18 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH v2] mtd: spi-nor: Fix shift-out-of-bounds in
 spi_nor_set_erase_type
Content-Language: en-US
To:     pratyush@kernel.org, michael@walle.cc,
        Alexander.Stein@tq-group.com, lrannou@baylibre.com
Cc:     linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
        stable@vger.kernel.org
References: <20230202191451.35142-1-tudor.ambarus@linaro.org>
From:   Tudor Ambarus <tudor.ambarus@linaro.org>
In-Reply-To: <20230202191451.35142-1-tudor.ambarus@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 247d1014879a..9b90d941d87a 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2019,10 +2019,22 @@ void spi_nor_set_erase_type(struct spi_nor_erase_type *erase, u32 size,
>   			    u8 opcode)
>   {
>   	erase->size = size;
> -	erase->opcode = opcode;
> -	/* JEDEC JESD216B Standard imposes erase sizes to be power of 2. */
> -	erase->size_shift = ffs(erase->size) - 1;
> -	erase->size_mask = (1 << erase->size_shift) - 1;
> +
> +	if (size) {

actually this is not needed now that spi_nor_mask_erase_type() is 
introduced. All the callers of spi_nor_set_erase_type() guarantee that
erase_size is not zero.

> +		erase->opcode = opcode;
> +		/* JEDEC JESD216B imposes erase sizes to be power of 2. */
> +		erase->size_shift = ffs(size) - 1;
> +		erase->size_mask = (1 << erase->size_shift) - 1;
> +	}
> +}
> +

So the fix should just contain the introduction of 
spi_nor_mask_erase_type(). Louis, do you want to authorship v3?

> +/**
> + * spi_nor_mask_erase_type() - mask out an SPI NOR erase type
> + * @erase:	pointer to a structure that describes a SPI NOR erase type

an SPI

> + */
> +void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase)
> +{
> +	erase->size = 0;
>   }
>   
>   /**
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index f6d012e1f681..25423225c29d 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -681,6 +681,7 @@ void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
>   
>   void spi_nor_set_erase_type(struct spi_nor_erase_type *erase, u32 size,
>   			    u8 opcode);
> +void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase);
>   struct spi_nor_erase_region *
>   spi_nor_region_next(struct spi_nor_erase_region *region);
>   void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map,
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index fd4daf8fa5df..298ab5e53a8c 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -875,7 +875,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>   	 */
>   	for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++)
>   		if (!(regions_erase_type & BIT(erase[i].idx)))
> -			spi_nor_set_erase_type(&erase[i], 0, 0xFF);
> +			spi_nor_mask_erase_type(&erase[i]);
>   
>   	return 0;
>   }
> @@ -1089,7 +1089,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
>   			erase_type[i].opcode = (dwords[SFDP_DWORD(2)] >>
>   						erase_type[i].idx * 8) & 0xFF;
>   		else
> -			spi_nor_set_erase_type(&erase_type[i], 0u, 0xFF);
> +			spi_nor_mask_erase_type(&erase_type[i]);
>   	}
>   
>   	/*