[PATCH v2] mtd: core: always verify OOB offset in mtd_check_oob_ops()

[Pratyush Yadav]

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 at ti.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