linux-kernel - Re: [PATCH v2] mtd: core: always verify OOB offset in mtd_check_oob

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: <mafs0v7lpi1j5.fsf@kernel.org>
Date: Thu, 11 Sep 2025 15:03:58 +0200
From: Pratyush Yadav <pratyush@...nel.org>
To: Gabor Juhos <j4g8y7@...il.com>
Cc: Miquel Raynal <miquel.raynal@...tlin.com>,  Santhosh Kumar K
 <s-k6@...com>,  Richard Weinberger <richard@....at>,  Vignesh Raghavendra
 <vigneshr@...com>,  linux-mtd@...ts.infradead.org,
  linux-kernel@...r.kernel.org,  stable@...r.kernel.org,  Daniel Golle
 <daniel@...rotopia.org>
Subject: Re: [PATCH v2] mtd: core: always verify OOB offset in
 mtd_check_oob_ops()

On Thu, Sep 11 2025, Gabor Juhos wrote:

> Hi Miquel, Santhosh,
>
> 2025. 09. 11. 10:00 keltezéssel, Miquel Raynal írta:
>> Hello,
>> 
>> On 11/09/2025 at 11:52:27 +0530, Santhosh Kumar K <s-k6@...com> wrote:
>> 
>>> Hello,
>>>
>>> On 05/09/25 20:25, Miquel Raynal wrote:
>>>> On Mon, 01 Sep 2025 16:24:35 +0200, Gabor Juhos wrote:
>>>>> Using an OOB offset past end of the available OOB data is invalid,
>>>>> irregardless of whether the 'ooblen' is set in the ops or not. Move
>>>>> the relevant check out from the if statement to always verify that.
>>>>>
>>>>> The 'oobtest' module executes four tests to verify how reading/writing
>>>>> OOB data past end of the devices is handled. It expects errors in case
>>>>> of these tests, but this expectation fails in the last two tests on
>>>>> MTD devices, which have no OOB bytes available.
>>>>>
>>>>> [...]
>>>> Applied to mtd/next, thanks!
>>>> [1/1] mtd: core: always verify OOB offset in mtd_check_oob_ops()
>>>>        commit: bf7d0543b2602be5cb450d8ec5a8710787806f88
>>>
>>> I'm seeing a failure in SPI NOR flashes due to this patch:
>>> (Tested on AM62x SK with S28HS512T OSPI NOR flash)
>
> Sorry for the inconvenience.
>
>> Gabor, can you check what happens with mtdblock?

My guess from a quick look at the code is that NOR devices have
mtd->oobsize == 0 and mtd_read() sets ops->ooboffs and ops->ooblen to 0.
So now that this check is not guarded by if (ops->ooblen), it gets
triggered for NOR devices on the mtd_read() path and essentially turns
into an if (0 >= 0), returning -EINVAL.

Maybe a better check is if ((ops->ooboffs + ops->ooblen) > mtd_oobavail())?

Note that the equality is not an error in this case. I haven't worked
with the OOB code much so I am not sure if this condition makes sense,
but seems to do so at first glance at least.

[...]

-- 
Regards,
Pratyush Yadav