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(-)
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c index 7900571fc85b..c35221794645 100644 --- a/drivers/mtd/nand/spi/core.c +++ b/drivers/mtd/nand/spi/core.c @@ -318,10 +318,6 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand, buf += ret; }
- if (req->ooblen) - memcpy(req->oobbuf.in, spinand->oobbuf + req->ooboffs, - req->ooblen); - return 0; }
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?
thanks,
greg k-h
On 2021-01-05 11:31, Greg KH wrote:
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?
I just checked, it seems that 4.19.y and 5.4.y are broken in exactly the same way.
On a first glance, it seems that the upstream commit has a wrong Fixes line and is fixing 3d1f08b032dc ("mtd: spinand: Use the external ECC engine logic") instead, which is not in 5.10. If that is correct, then we don't need any stable backport at all.
In my opinion it's best to just revert the broken commit in all the stable trees as quickly as possible and let Miquel sort out the mess afterwards, if needed.
- Felix
On Tue, Jan 05, 2021 at 11:40:11AM +0100, Felix Fietkau wrote:
On 2021-01-05 11:31, Greg KH wrote:
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?
I just checked, it seems that 4.19.y and 5.4.y are broken in exactly the same way.
On a first glance, it seems that the upstream commit has a wrong Fixes line and is fixing 3d1f08b032dc ("mtd: spinand: Use the external ECC engine logic") instead, which is not in 5.10. If that is correct, then we don't need any stable backport at all.
Ah, then yes, this should be reverted everywhere. If that is the case, otherwise, I went off of the fixes: value in the original commit, which is in the 4.19.0 release.
In my opinion it's best to just revert the broken commit in all the stable trees as quickly as possible and let Miquel sort out the mess afterwards, if needed.
I can do that, but need an ack from the maintainer...
thanks,
greg k-h
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.
Thanks, Miquèl
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.
- Felix
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
linux-stable-mirror@lists.linaro.org
-
Felix Fietkau -
Greg KH -
Miquel Raynal