linux-kernel - Re: [PATCH] mtd: spi-nor: Enable locking for n25q00a

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: <26a795ac-e6ff-4363-a8b9-38793a9be794@linux.dev>
Date: Thu, 9 Oct 2025 18:27:35 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: Pratyush Yadav <pratyush@...nel.org>
Cc: Tudor Ambarus <tudor.ambarus@...aro.org>,
 Michael Walle <mwalle@...nel.org>, linux-mtd@...ts.infradead.org,
 Richard Weinberger <richard@....at>, linux-kernel@...r.kernel.org,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH] mtd: spi-nor: Enable locking for n25q00a

On 10/8/25 08:30, Pratyush Yadav wrote:
> On Tue, Oct 07 2025, Sean Anderson wrote:
> 
>> On 10/7/25 09:15, Pratyush Yadav wrote:
>>> On Mon, Oct 06 2025, Sean Anderson wrote:
>>> 
>>>> The datasheet for n25q00a shows that the status register has the same
>>>> layout as for n25q00, so use the same flags to enable locking support.
>>>> These flags should have been added back in commit 150ccc181588 ("mtd:
>>>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>>>> maintainer...
>>> 
>>> This makes it sound like the maintainer did something wrong, which is
>>> not true. Tudor had a good reason for removing them.
>>
>> I disagree. The maintainer used his position of authority to make the
>> submitter second-guess their correct patch.
> 
> Sean, you are being very combative over such a small issue. You must
> test your changes. This is one of the most basic principles in software
> engineering. It was perfectly reasonable from Tudor to push back on
> untested changes.
> 
> There is no abuse of "position of authority" here. When things break, we
> get to do the work of putting the pieces back together. So of course, we
> are reluctant to take things that increase this burden for us. Having
> contributors test their changes is the simplest of things we ask for to
> keep the quality bar.
> 
> Beyond that, I'd say that a little politeness goes a long way in life.
> Especially towards the people maintaining the software for free that you
> (or your employer) use. We are both wasting our energy on this debate.
> Please stop. Take a step back and think from the other side's
> perspective. And try to work _with_ people, not against them.
> 
>>
>> These flashes have capacity of greater than the 8 MiB that can be
>> protected using 3 BP bits. Micron (and ST before them?) addressed this
>> by adding a fourth BP bit. This is consistent across every flash in this
>> series, and is clearly documented in every datasheet. Defaulting to 3
>> bits is buggy behavior: we should assume flashes behave per their
>> datasheets until proven otherwise, especially for less-popular features
> 
> If I had a euro every time I found a bug in a datasheet, well, I would
> have enough money to at least buy a nice dinner. My point is, datasheets
> are not perfect. Only running on real hardware gets you the true
> picture.

Well, it's even *more* buggy to pretend that the datasheet doesn't exist
and just do whatever you please. Might as well reverse-engineer every
chip that comes across your desk from first principles with that
attitude.

The locking doesn't work on any of these flashes without these flags. If
you don't believe me you can try it yourself. The people who submitted
the original patches certainly didn't test it.

--Sean