Re: [PATCH] Revert "mtd: spinand: Fix OOB read"

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Felix,

Felix Fietkau <nbd@nbd.name> wrote on Tue, 5 Jan 2021 11:47:27 +0100:

> On 2021-01-05 11:40, Miquel Raynal wrote:
> > Hello,
> > 
> > Greg KH <gregkh@linuxfoundation.org> wrote on Tue, 5 Jan 2021 11:31:26
> > +0100:
> >   
> >> On Tue, Jan 05, 2021 at 11:18:21AM +0100, Felix Fietkau wrote:  
> >> > This reverts stable commit baad618d078c857f99cc286ea249e9629159901f.
> >> > 
> >> > This commit is adding lines to spinand_write_to_cache_op, wheras the upstream
> >> > commit 868cbe2a6dcee451bd8f87cbbb2a73cf463b57e5 that this was supposed to
> >> > backport was touching spinand_read_from_cache_op.
> >> > It causes a crash on writing OOB data by attempting to write to read-only
> >> > kernel memory.
> >> > 
> >> > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> >> > Signed-off-by: Felix Fietkau <nbd@nbd.name>
> >> > ---
> >> >  drivers/mtd/nand/spi/core.c | 4 ----
> >> >  1 file changed, 4 deletions(-)    
> >> 
> >> So the backport to 5.10.y broke, but not the backport to 4.19.y or
> >> 5.4.y?  Can you provide a "correct" backport for this instead of just
> >> removing this fix?  
> > 
> > Agreed, I think the proper way to handle the situation would be to move
> > these three lines to spinand_read_from_cache_op() instead of just
> > getting rid of them.  
> But they have a similar line in spinand_read_from_cache_op already (in
> addition to some extra code for dealing with MTD_OPS_AUTO_OOB).
> 
> Please take another look at your commit 3d1f08b032dc, it really looks to
> me like you were just adding back a part of what you removed there.
> Maybe the proper solution is to add back the rest of it as well (the
> part that deals with MTD_OPS_AUTO_OOB) and leave the stable kernels alone.

You are actually right, I've got confused by my own previous change.

The right thing to do is indeed to get rid of this patch in the stable
kernels.

Thanks for reporting,
Miquèl