[PATCH] mtd: spi-nor: sst: Fix SST write failure

[Tudor Ambarus]

On 2/13/25 7:47 AM, Pratyush Yadav wrote:
> Hi Amit,
> 
> On Thu, Feb 13 2025, Amit Kumar Mahapatra wrote:
> 
>> 'commit 18bcb4aa54ea ("mtd: spi-nor: sst: Factor out common write operation
>> to `sst_nor_write_data()`")' introduced a bug where only one byte of data
>> is written, regardless of the number of bytes passed to
>> sst_nor_write_data(), causing a kernel crash during the write operation.
> 
> Does it crash or only cause a warning? This comes from a WARN() so it
> should not crash the kernel unless you set panic_on_warn.
> 
>> Ensure the correct number of bytes are written as passed to
>> sst_nor_write_data().
>>
>> Call trace:
>> [   57.400180] ------------[ cut here ]------------
>> [   57.404842] While writing 2 byte written 1 bytes
>> [   57.409493] WARNING: CPU: 0 PID: 737 at drivers/mtd/spi-nor/sst.c:187 sst_nor_write_data+0x6c/0x74
>> [   57.418464] Modules linked in:
>> [   57.421517] CPU: 0 UID: 0 PID: 737 Comm: mtd_debug Not tainted 6.12.0-g5ad04afd91f9 #30
>> [   57.429517] Hardware name: Xilinx Versal A2197 Processor board revA - x-prc-02 revA (DT)
>> [   57.437600] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [   57.444557] pc : sst_nor_write_data+0x6c/0x74
>> [   57.448911] lr : sst_nor_write_data+0x6c/0x74
>> [   57.453264] sp : ffff80008232bb40
>> [   57.456570] x29: ffff80008232bb40 x28: 0000000000010000 x27: 0000000000000001
>> [   57.463708] x26: 000000000000ffff x25: 0000000000000000 x24: 0000000000000000
>> [   57.470843] x23: 0000000000010000 x22: ffff80008232bbf0 x21: ffff000816230000
>> [   57.477978] x20: ffff0008056c0080 x19: 0000000000000002 x18: 0000000000000006
>> [   57.485112] x17: 0000000000000000 x16: 0000000000000000 x15: ffff80008232b580
>> [   57.492246] x14: 0000000000000000 x13: ffff8000816d1530 x12: 00000000000004a4
>> [   57.499380] x11: 000000000000018c x10: ffff8000816fd530 x9 : ffff8000816d1530
>> [   57.506515] x8 : 00000000fffff7ff x7 : ffff8000816fd530 x6 : 0000000000000001
>> [   57.513649] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
>> [   57.520782] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008049b0000
>> [   57.527916] Call trace:
>> [   57.530354]  sst_nor_write_data+0x6c/0x74
>> [   57.534361]  sst_nor_write+0xb4/0x18c
>> [   57.538019]  mtd_write_oob_std+0x7c/0x88
>> [   57.541941]  mtd_write_oob+0x70/0xbc
>> [   57.545511]  mtd_write+0x68/0xa8
>> [   57.548733]  mtdchar_write+0x10c/0x290
>> [   57.552477]  vfs_write+0xb4/0x3a8
>> [   57.555791]  ksys_write+0x74/0x10c
>> [   57.559189]  __arm64_sys_write+0x1c/0x28
>> [   57.563109]  invoke_syscall+0x54/0x11c
>> [   57.566856]  el0_svc_common.constprop.0+0xc0/0xe0
>> [   57.571557]  do_el0_svc+0x1c/0x28
>> [   57.574868]  el0_svc+0x30/0xcc
>> [   57.577921]  el0t_64_sync_handler+0x120/0x12c
>> [   57.582276]  el0t_64_sync+0x190/0x194
>> [   57.585933] ---[ end trace 0000000000000000 ]---
>>
>> Fixes: 18bcb4aa54ea ("mtd: spi-nor: sst: Factor out common write operation to `sst_nor_write_data()`")
>> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra at amd.com>
>> ---
>> BRANCH: mtd/next
>> ---
>>  drivers/mtd/spi-nor/sst.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/sst.c b/drivers/mtd/spi-nor/sst.c
>> index b5ad7118c49a..175211fe6a5e 100644
>> --- a/drivers/mtd/spi-nor/sst.c
>> +++ b/drivers/mtd/spi-nor/sst.c
>> @@ -174,7 +174,7 @@ static int sst_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>>  	int ret;
>>  
>>  	nor->program_opcode = op;
>> -	ret = spi_nor_write_data(nor, to, 1, buf);
>> +	ret = spi_nor_write_data(nor, to, len, buf);
> 
> What a silly bug! Thanks for fixing it. I will let the patch sit on the
> list for a couple days to get some eyes on it and then will apply it.

Looks good. Needs Cc to stable, it was introduced in v6.12, and we can
get it as fixes for v6.14. With cc to stable:

Reviewed-by: Tudor Ambarus <tudor.ambarus at linaro.org>

> 
> Reviewed-by: Pratyush Yadav <pratyush at kernel.org>
> 
>>  	if (ret < 0)
>>  		return ret;
>>  	WARN(ret != len, "While writing %zu byte written %i bytes\n", len, ret);
>