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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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);
> }
>
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 <[email protected]>
Good catch.
Reviewed-by: Baolin Wang <[email protected]>
> ---
> 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);
>
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 <[email protected]>
> ---
> 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);
>