linux-kernel - Re: [PATCH] spi-nor: Verify written data in paranoid mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dd3c7cc0-a568-4046-b105-e6786b5c80f8@prolan.hu>
Date: Wed, 16 Apr 2025 14:38:27 +0200
From: Csókás Bence <csokas.bence@...lan.hu>
To: Michael Walle <mwalle@...nel.org>, <linux-mtd@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>
CC: Szentendrei, Tamás <szentendrei.tamas@...lan.hu>,
	"Tudor Ambarus" <tudor.ambarus@...aro.org>, Pratyush Yadav
	<pratyush@...nel.org>, Miquel Raynal <miquel.raynal@...tlin.com>, Richard
 Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH] spi-nor: Verify written data in paranoid mode

Hi,

On 2025. 04. 16. 13:59, Michael Walle wrote:
> Hi,
> 
>> Add MTD_SPI_NOR_PARANOID config option for verifying all written data to
>> prevent silent bit errors to be undetected, at the cost of halving SPI
>> bandwidth.
> 
> What is the use case for this? Why is it specific to SPI-NOR
> flashes? Or should it rather be an MTD "feature". I'm not sure
> whether this is the right way to do it, thus I'd love to hear more
> about the background story to this.

Well, our case is quite specific, but we wanted to provide a general 
solution for upstream. In our case we have a component in the data path 
that can cause a burst bit error, on average after about a hundred 
megabytes written.

We _could_ make it MTD-wide, in our case we only have a NOR Flash 
onboard so this is where we added it. If it were in the MTD core, where 
would it make sense?

* mtd_write()
* mtd_write_oob()
* mtd_write_oob_std()
* or somewhere else entirely?

>> Co-developed-by: Szentendrei, Tamás <szentendrei.tamas@...lan.hu>
>> Signed-off-by: Szentendrei, Tamás <szentendrei.tamas@...lan.hu>
>> Signed-off-by: Csókás, Bence <csokas.bence@...lan.hu>
>> ---
>>   drivers/mtd/spi-nor/Kconfig | 10 ++++++++++
>>   drivers/mtd/spi-nor/core.c  | 33 +++++++++++++++++++++++++++++++++
>>   2 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 24cd25de2b8b..425ea9a22424 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -68,6 +68,16 @@ config MTD_SPI_NOR_SWP_KEEP
>>   
>>   endchoice
>>   
>> +config MTD_SPI_NOR_PARANOID
>> +	bool "Read back written data (paranoid mode)"
> 
> No kernel configs please. This doesn't scale. What if you have two
> flashes and one should have this and one does not?

Yes, we have thought about this, but concluded that "paranoid mode" is 
not device-specific, you either have a requirement to be extra sure 
about data integrity, or you can be fairly sure your system is sane, in 
both cases it is a judgement about the entire system. I also thought 
that maybe some devices could be exempt from this, contingent on a 
`no-paranoia` Device Tree property, to selectively sacrifice integrity 
for performance even in paranoid mode, but we only have one Flash 
anyway, so I didn't implement it.

> -michael

Bence