[PATCH 1/3] nvmem: sprd: Fix memory overflow issue during memcpy operation in efuse driver

[PATCH 1/3] nvmem: sprd: Fix memory overflow issue during memcpy operation in efuse driver

[Yanxin Huang]

The efuse driver didn't determine the size of bytes, resulting in memory
overflow during memcpy operation.

Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
Signed-off-by: Yanxin Huang <yanxin.huang@unisoc.com>
---
 drivers/nvmem/sprd-efuse.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
index bb3105f3291f..24b63620d217 100644
--- a/drivers/nvmem/sprd-efuse.c
+++ b/drivers/nvmem/sprd-efuse.c
@@ -311,6 +311,7 @@ static int sprd_efuse_read(void *context, u32 offset, void *val, size_t bytes)
 	ret = sprd_efuse_raw_read(efuse, index, &data, blk_double);
 	if (!ret) {
 		data >>= blk_offset;
+		bytes = bytes > sizeof(data) ? sizeof(data) : bytes;
 		memcpy(val, &data, bytes);
 	}
 
-- 
2.17.1

[PATCH 2/3] nvmem: sprd: Fix programming errors in efuse caused by incorrect parameters

[Yanxin Huang]

The second argument to sprd_efuse_raw_prog() is the efuse index block
data, but the data passed in is the efuse block offset, which can cause
efuse to be programmed to the wrong block.

Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
Signed-off-by: Yanxin Huang <yanxin.huang@unisoc.com>
---
 drivers/nvmem/sprd-efuse.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
index 24b63620d217..f0880f8fc56d 100644
--- a/drivers/nvmem/sprd-efuse.c
+++ b/drivers/nvmem/sprd-efuse.c
@@ -326,6 +326,7 @@ static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
 {
 	struct sprd_efuse *efuse = context;
 	bool blk_double = efuse->data->blk_double;
+	u32 index = offset / SPRD_EFUSE_BLOCK_WIDTH + efuse->data->blk_offset;
 	bool lock;
 	int ret;
 
@@ -350,7 +351,7 @@ static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
 	else
 		lock = true;
 
-	ret = sprd_efuse_raw_prog(efuse, offset, blk_double, lock, val);
+	ret = sprd_efuse_raw_prog(efuse, index, blk_double, lock, val);
 
 	clk_disable_unprepare(efuse->clk);
 
-- 
2.17.1

Re: [PATCH 2/3] nvmem: sprd: Fix programming errors in efuse caused by incorrect parameters

[Baolin Wang]

On 12/8/2023 2:11 PM, Yanxin Huang wrote:
> The second argument to sprd_efuse_raw_prog() is the efuse index block
> data, but the data passed in is the efuse block offset, which can cause
> efuse to be programmed to the wrong block.
> 
> Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")
> Signed-off-by: Yanxin Huang <yanxin.huang@unisoc.com>

Good catch.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

> ---
>   drivers/nvmem/sprd-efuse.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> index 24b63620d217..f0880f8fc56d 100644
> --- a/drivers/nvmem/sprd-efuse.c
> +++ b/drivers/nvmem/sprd-efuse.c
> @@ -326,6 +326,7 @@ static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
>   {
>   	struct sprd_efuse *efuse = context;
>   	bool blk_double = efuse->data->blk_double;
> +	u32 index = offset / SPRD_EFUSE_BLOCK_WIDTH + efuse->data->blk_offset;
>   	bool lock;
>   	int ret;
>   
> @@ -350,7 +351,7 @@ static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
>   	else
>   		lock = true;
>   
> -	ret = sprd_efuse_raw_prog(efuse, offset, blk_double, lock, val);
> +	ret = sprd_efuse_raw_prog(efuse, index, blk_double, lock, val);
>   
>   	clk_disable_unprepare(efuse->clk);
>   

[PATCH 3/3] nvmem: sprd: Remove the lock operation to support customers being able to program efuse multiple times

[Yanxin Huang]

The customer uses the efuse interface to program efuse based on block
size. Each time a part of the content is programmed, according to the
original code logic, as long as the bytes parameter is equal to the block
size, the block will be locked, which will result in the efuse block
being unable to program multiple times.

This patch removes the efuse block locking operation, as the unisoc efuse
driver supports customers to program the same block multiple times.If you
need to lock a block, you can directly program the lock bit of the block.

Signed-off-by: Yanxin Huang <yanxin.huang@unisoc.com>
---
 drivers/nvmem/sprd-efuse.c | 57 ++------------------------------------
 1 file changed, 2 insertions(+), 55 deletions(-)

diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
index f0880f8fc56d..5220fd680f47 100644
--- a/drivers/nvmem/sprd-efuse.c
+++ b/drivers/nvmem/sprd-efuse.c
@@ -143,30 +143,6 @@ static void sprd_efuse_set_read_power(struct sprd_efuse *efuse, bool en)
 	usleep_range(1000, 1200);
 }
 
-static void sprd_efuse_set_prog_lock(struct sprd_efuse *efuse, bool en)
-{
-	u32 val = readl(efuse->base + SPRD_EFUSE_ENABLE);
-
-	if (en)
-		val |= SPRD_EFUSE_LOCK_WR_EN;
-	else
-		val &= ~SPRD_EFUSE_LOCK_WR_EN;
-
-	writel(val, efuse->base + SPRD_EFUSE_ENABLE);
-}
-
-static void sprd_efuse_set_auto_check(struct sprd_efuse *efuse, bool en)
-{
-	u32 val = readl(efuse->base + SPRD_EFUSE_ENABLE);
-
-	if (en)
-		val |= SPRD_EFUSE_AUTO_CHECK_EN;
-	else
-		val &= ~SPRD_EFUSE_AUTO_CHECK_EN;
-
-	writel(val, efuse->base + SPRD_EFUSE_ENABLE);
-}
-
 static void sprd_efuse_set_data_double(struct sprd_efuse *efuse, bool en)
 {
 	u32 val = readl(efuse->base + SPRD_EFUSE_ENABLE);
@@ -191,8 +167,7 @@ static void sprd_efuse_set_prog_en(struct sprd_efuse *efuse, bool en)
 	writel(val, efuse->base + SPRD_EFUSE_PW_SWT);
 }
 
-static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
-			       bool lock, u32 *data)
+static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub, u32 *data)
 {
 	u32 status;
 	int ret = 0;
@@ -213,18 +188,8 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
 	sprd_efuse_set_prog_en(efuse, true);
 	sprd_efuse_set_data_double(efuse, doub);
 
-	/*
-	 * Enable the auto-check function to validate if the programming is
-	 * successful.
-	 */
-	if (lock)
-		sprd_efuse_set_auto_check(efuse, true);
-
 	writel(*data, efuse->base + SPRD_EFUSE_MEM(blk));
 
-	/* Disable auto-check and data double after programming */
-	if (lock)
-		sprd_efuse_set_auto_check(efuse, false);
 	sprd_efuse_set_data_double(efuse, false);
 
 	/*
@@ -239,10 +204,6 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
 		writel(SPRD_EFUSE_ERR_CLR_MASK,
 		       efuse->base + SPRD_EFUSE_ERR_CLR);
 		ret = -EBUSY;
-	} else if (lock) {
-		sprd_efuse_set_prog_lock(efuse, lock);
-		writel(0, efuse->base + SPRD_EFUSE_MEM(blk));
-		sprd_efuse_set_prog_lock(efuse, false);
 	}
 
 	sprd_efuse_set_prog_power(efuse, false);
@@ -327,7 +288,6 @@ static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
 	struct sprd_efuse *efuse = context;
 	bool blk_double = efuse->data->blk_double;
 	u32 index = offset / SPRD_EFUSE_BLOCK_WIDTH + efuse->data->blk_offset;
-	bool lock;
 	int ret;
 
 	ret = sprd_efuse_lock(efuse);
@@ -338,20 +298,7 @@ static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
 	if (ret)
 		goto unlock;
 
-	/*
-	 * If the writing bytes are equal with the block width, which means the
-	 * whole block will be programmed. For this case, we should not allow
-	 * this block to be programmed again by locking this block.
-	 *
-	 * If the block was programmed partially, we should allow this block to
-	 * be programmed again.
-	 */
-	if (bytes < SPRD_EFUSE_BLOCK_WIDTH)
-		lock = false;
-	else
-		lock = true;
-
-	ret = sprd_efuse_raw_prog(efuse, index, blk_double, lock, val);
+	ret = sprd_efuse_raw_prog(efuse, index, blk_double, val);
 
 	clk_disable_unprepare(efuse->clk);
 
-- 
2.17.1

Re: [PATCH 3/3] nvmem: sprd: Remove the lock operation to support customers being able to program efuse multiple times

[Baolin Wang]

On 12/8/2023 2:11 PM, Yanxin Huang wrote:
> The customer uses the efuse interface to program efuse based on block
> size. Each time a part of the content is programmed, according to the
> original code logic, as long as the bytes parameter is equal to the block
> size, the block will be locked, which will result in the efuse block
> being unable to program multiple times.

Initially, we only supported one-time programming. Can you describe the 
scenarios for multiple programming?

> This patch removes the efuse block locking operation, as the unisoc efuse
> driver supports customers to program the same block multiple times.If you
> need to lock a block, you can directly program the lock bit of the block.

How can "directly program the lock bit of the block" for users? You 
already removed sprd_efuse_set_prog_lock().


BTW, You should separate the patches for bugfix and feature 
modifications into different patch sets, so that the bugfix patches can 
be reviewed and merged ASAP.

> Signed-off-by: Yanxin Huang <yanxin.huang@unisoc.com>
> ---
>   drivers/nvmem/sprd-efuse.c | 57 ++------------------------------------
>   1 file changed, 2 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> index f0880f8fc56d..5220fd680f47 100644
> --- a/drivers/nvmem/sprd-efuse.c
> +++ b/drivers/nvmem/sprd-efuse.c
> @@ -143,30 +143,6 @@ static void sprd_efuse_set_read_power(struct sprd_efuse *efuse, bool en)
>   	usleep_range(1000, 1200);
>   }
>   
> -static void sprd_efuse_set_prog_lock(struct sprd_efuse *efuse, bool en)
> -{
> -	u32 val = readl(efuse->base + SPRD_EFUSE_ENABLE);
> -
> -	if (en)
> -		val |= SPRD_EFUSE_LOCK_WR_EN;
> -	else
> -		val &= ~SPRD_EFUSE_LOCK_WR_EN;
> -
> -	writel(val, efuse->base + SPRD_EFUSE_ENABLE);
> -}
> -
> -static void sprd_efuse_set_auto_check(struct sprd_efuse *efuse, bool en)
> -{
> -	u32 val = readl(efuse->base + SPRD_EFUSE_ENABLE);
> -
> -	if (en)
> -		val |= SPRD_EFUSE_AUTO_CHECK_EN;
> -	else
> -		val &= ~SPRD_EFUSE_AUTO_CHECK_EN;
> -
> -	writel(val, efuse->base + SPRD_EFUSE_ENABLE);
> -}
> -
>   static void sprd_efuse_set_data_double(struct sprd_efuse *efuse, bool en)
>   {
>   	u32 val = readl(efuse->base + SPRD_EFUSE_ENABLE);
> @@ -191,8 +167,7 @@ static void sprd_efuse_set_prog_en(struct sprd_efuse *efuse, bool en)
>   	writel(val, efuse->base + SPRD_EFUSE_PW_SWT);
>   }
>   
> -static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
> -			       bool lock, u32 *data)
> +static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub, u32 *data)
>   {
>   	u32 status;
>   	int ret = 0;
> @@ -213,18 +188,8 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
>   	sprd_efuse_set_prog_en(efuse, true);
>   	sprd_efuse_set_data_double(efuse, doub);
>   
> -	/*
> -	 * Enable the auto-check function to validate if the programming is
> -	 * successful.
> -	 */
> -	if (lock)
> -		sprd_efuse_set_auto_check(efuse, true);
> -
>   	writel(*data, efuse->base + SPRD_EFUSE_MEM(blk));
>   
> -	/* Disable auto-check and data double after programming */
> -	if (lock)
> -		sprd_efuse_set_auto_check(efuse, false);
>   	sprd_efuse_set_data_double(efuse, false);
>   
>   	/*
> @@ -239,10 +204,6 @@ static int sprd_efuse_raw_prog(struct sprd_efuse *efuse, u32 blk, bool doub,
>   		writel(SPRD_EFUSE_ERR_CLR_MASK,
>   		       efuse->base + SPRD_EFUSE_ERR_CLR);
>   		ret = -EBUSY;
> -	} else if (lock) {
> -		sprd_efuse_set_prog_lock(efuse, lock);
> -		writel(0, efuse->base + SPRD_EFUSE_MEM(blk));
> -		sprd_efuse_set_prog_lock(efuse, false);
>   	}
>   
>   	sprd_efuse_set_prog_power(efuse, false);
> @@ -327,7 +288,6 @@ static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
>   	struct sprd_efuse *efuse = context;
>   	bool blk_double = efuse->data->blk_double;
>   	u32 index = offset / SPRD_EFUSE_BLOCK_WIDTH + efuse->data->blk_offset;
> -	bool lock;
>   	int ret;
>   
>   	ret = sprd_efuse_lock(efuse);
> @@ -338,20 +298,7 @@ static int sprd_efuse_write(void *context, u32 offset, void *val, size_t bytes)
>   	if (ret)
>   		goto unlock;
>   
> -	/*
> -	 * If the writing bytes are equal with the block width, which means the
> -	 * whole block will be programmed. For this case, we should not allow
> -	 * this block to be programmed again by locking this block.
> -	 *
> -	 * If the block was programmed partially, we should allow this block to
> -	 * be programmed again.
> -	 */
> -	if (bytes < SPRD_EFUSE_BLOCK_WIDTH)
> -		lock = false;
> -	else
> -		lock = true;
> -
> -	ret = sprd_efuse_raw_prog(efuse, index, blk_double, lock, val);
> +	ret = sprd_efuse_raw_prog(efuse, index, blk_double, val);
>   
>   	clk_disable_unprepare(efuse->clk);
>   

Re: [PATCH 1/3] nvmem: sprd: Fix memory overflow issue during memcpy operation in efuse driver

[Srinivas Kandagatla]

thanks for the patch.

On 08/12/2023 06:11, Yanxin Huang wrote:
> The efuse driver didn't determine the size of bytes, resulting in memory
> overflow during memcpy operation.
> 
> Fixes: 096030e7f449 ("nvmem: sprd: Add Spreadtrum SoCs eFuse support")

Please add CC Stable for all the fixes.

> Signed-off-by: Yanxin Huang <yanxin.huang@unisoc.com>
> ---
>   drivers/nvmem/sprd-efuse.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nvmem/sprd-efuse.c b/drivers/nvmem/sprd-efuse.c
> index bb3105f3291f..24b63620d217 100644
> --- a/drivers/nvmem/sprd-efuse.c
> +++ b/drivers/nvmem/sprd-efuse.c
> @@ -311,6 +311,7 @@ static int sprd_efuse_read(void *context, u32 offset, void *val, size_t bytes)
>   	ret = sprd_efuse_raw_read(efuse, index, &data, blk_double);
>   	if (!ret) {
>   		data >>= blk_offset;
> +		bytes = bytes > sizeof(data) ? sizeof(data) : bytes;

looks like sprd_efuse_read is only reading upto 4 bytes max, and 
silently ignoring reading requests data after 4 bytes.

Is this working because consumers so far only requested 4 bytes and less ?
does dumping nvmem sysfs actually show real content in your case?

--srini
>   		memcpy(val, &data, bytes);


>   	}
>