| Message ID | 20181223003126.1704-1-chunkeey@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Boris Brezillon |
| Headers | show |
| Series | mtd: rawnand: qcom: fix memory corruption that causes panic | expand |
Hi Christian, Christian Lamparter <chunkeey@gmail.com> wrote on Sun, 23 Dec 2018 01:31:26 +0100: > This patch fixes a memory corruption that occurred in the > qcom-nandc driver since it was converted to nand_scan(). > > On boot, an affected device will panic from a NPE at a weird place: > | Unable to handle kernel NULL pointer dereference at virtual address 0 > | pgd = (ptrval) > | [00000000] *pgd=00000000 > | Internal error: Oops: 80000005 [#1] SMP ARM > | CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.9 #0 > | Hardware name: Generic DT based system > | PC is at (null) > | LR is at nand_block_isbad+0x90/0xa4 > | pc : [<00000000>] lr : [<c0592240>] psr: 80000013 > | sp : cf839d40 ip : 00000000 fp : cfae9e20 > | r10: cf815810 r9 : 00000000 r8 : 00000000 > | r7 : 00000000 r6 : 00000000 r5 : 00000001 r4 : cf815810 > | r3 : 00000000 r2 : cfae9810 r1 : ffffffff r0 : cf815810 > | Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > | Control: 10c5387d Table: 8020406a DAC: 00000051 > | Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) > | [<c0592240>] (nand_block_isbad) from [<c0580a94>] > | [<c0580a94>] (allocate_partition) from [<c05811e4>] > | [<c05811e4>] (add_mtd_partitions) from [<c0581164>] > | [<c0581164>] (parse_mtd_partitions) from [<c057def4>] > | [<c057def4>] (mtd_device_parse_register) from [<c059d274>] > | [<c059d274>] (qcom_nandc_probe) from [<c0567f00>] > > The problem is that the nand_scan()'s qcom_nand_attach_chip callback > is updating the nandc->max_cwperpage from 1 to 4. This causes the > sg_init_table of clear_bam_transaction() in the driver's > qcom_nandc_block_bad() to memset much more than what was initially > allocated by alloc_bam_transaction(). > > This patch restores the old behavior by reallocating the shared bam > transaction alloc_bam_transaction() after the chip was identified, > but before mtd_device_parse_register() (which is an alias for > mtd_device_register() - see panic) gets called. This fixes the > corruption and the driver is working again. > > Cc: stable@vger.kernel.org > Fixes: 6a3cec64f18c ("mtd: rawnand: qcom: convert driver to nand_scan()") > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > --- Acked-by: Miquel Raynal <miquel.raynal@bootlin.com> Thanks, Miquèl
From: Boris Brezillon <boris.brezillon@bootlin.com> On Sun, 2018-12-23 at 00:31:26 UTC, Christian Lamparter wrote: > This patch fixes a memory corruption that occurred in the > qcom-nandc driver since it was converted to nand_scan(). > > On boot, an affected device will panic from a NPE at a weird place: > | Unable to handle kernel NULL pointer dereference at virtual address 0 > | pgd = (ptrval) > | [00000000] *pgd=00000000 > | Internal error: Oops: 80000005 [#1] SMP ARM > | CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.9 #0 > | Hardware name: Generic DT based system > | PC is at (null) > | LR is at nand_block_isbad+0x90/0xa4 > | pc : [<00000000>] lr : [<c0592240>] psr: 80000013 > | sp : cf839d40 ip : 00000000 fp : cfae9e20 > | r10: cf815810 r9 : 00000000 r8 : 00000000 > | r7 : 00000000 r6 : 00000000 r5 : 00000001 r4 : cf815810 > | r3 : 00000000 r2 : cfae9810 r1 : ffffffff r0 : cf815810 > | Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > | Control: 10c5387d Table: 8020406a DAC: 00000051 > | Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) > | [<c0592240>] (nand_block_isbad) from [<c0580a94>] > | [<c0580a94>] (allocate_partition) from [<c05811e4>] > | [<c05811e4>] (add_mtd_partitions) from [<c0581164>] > | [<c0581164>] (parse_mtd_partitions) from [<c057def4>] > | [<c057def4>] (mtd_device_parse_register) from [<c059d274>] > | [<c059d274>] (qcom_nandc_probe) from [<c0567f00>] > > The problem is that the nand_scan()'s qcom_nand_attach_chip callback > is updating the nandc->max_cwperpage from 1 to 4. This causes the > sg_init_table of clear_bam_transaction() in the driver's > qcom_nandc_block_bad() to memset much more than what was initially > allocated by alloc_bam_transaction(). > > This patch restores the old behavior by reallocating the shared bam > transaction alloc_bam_transaction() after the chip was identified, > but before mtd_device_parse_register() (which is an alias for > mtd_device_register() - see panic) gets called. This fixes the > corruption and the driver is working again. > > Cc: stable@vger.kernel.org > Fixes: 6a3cec64f18c ("mtd: rawnand: qcom: convert driver to nand_scan()") > Signed-off-by: Christian Lamparter <chunkeey@gmail.com> > Acked-by: Miquel Raynal <miquel.raynal@bootlin.com> Applied to http://git.infradead.org/linux-mtd.git mtd/fixes, thanks. Boris
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c index 699d3cf49c6d..7c42a57aca1f 100644 --- a/drivers/mtd/nand/raw/qcom_nandc.c +++ b/drivers/mtd/nand/raw/qcom_nandc.c @@ -2833,6 +2833,16 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc, if (ret) return ret; + if (nandc->props->is_bam) { + free_bam_transaction(nandc); + nandc->bam_txn = alloc_bam_transaction(nandc); + if (!nandc->bam_txn) { + dev_err(nandc->dev, + "failed to allocate bam transaction\n"); + return -ENOMEM; + } + } + ret = mtd_device_register(mtd, NULL, 0); if (ret) nand_cleanup(chip); @@ -2847,16 +2857,6 @@ static int qcom_probe_nand_devices(struct qcom_nand_controller *nandc) struct qcom_nand_host *host; int ret; - if (nandc->props->is_bam) { - free_bam_transaction(nandc); - nandc->bam_txn = alloc_bam_transaction(nandc); - if (!nandc->bam_txn) { - dev_err(nandc->dev, - "failed to allocate bam transaction\n"); - return -ENOMEM; - } - } - for_each_available_child_of_node(dn, child) { host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL); if (!host) {
This patch fixes a memory corruption that occurred in the qcom-nandc driver since it was converted to nand_scan(). On boot, an affected device will panic from a NPE at a weird place: | Unable to handle kernel NULL pointer dereference at virtual address 0 | pgd = (ptrval) | [00000000] *pgd=00000000 | Internal error: Oops: 80000005 [#1] SMP ARM | CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.19.9 #0 | Hardware name: Generic DT based system | PC is at (null) | LR is at nand_block_isbad+0x90/0xa4 | pc : [<00000000>] lr : [<c0592240>] psr: 80000013 | sp : cf839d40 ip : 00000000 fp : cfae9e20 | r10: cf815810 r9 : 00000000 r8 : 00000000 | r7 : 00000000 r6 : 00000000 r5 : 00000001 r4 : cf815810 | r3 : 00000000 r2 : cfae9810 r1 : ffffffff r0 : cf815810 | Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none | Control: 10c5387d Table: 8020406a DAC: 00000051 | Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) | [<c0592240>] (nand_block_isbad) from [<c0580a94>] | [<c0580a94>] (allocate_partition) from [<c05811e4>] | [<c05811e4>] (add_mtd_partitions) from [<c0581164>] | [<c0581164>] (parse_mtd_partitions) from [<c057def4>] | [<c057def4>] (mtd_device_parse_register) from [<c059d274>] | [<c059d274>] (qcom_nandc_probe) from [<c0567f00>] The problem is that the nand_scan()'s qcom_nand_attach_chip callback is updating the nandc->max_cwperpage from 1 to 4. This causes the sg_init_table of clear_bam_transaction() in the driver's qcom_nandc_block_bad() to memset much more than what was initially allocated by alloc_bam_transaction(). This patch restores the old behavior by reallocating the shared bam transaction alloc_bam_transaction() after the chip was identified, but before mtd_device_parse_register() (which is an alias for mtd_device_register() - see panic) gets called. This fixes the corruption and the driver is working again. Cc: stable@vger.kernel.org Fixes: 6a3cec64f18c ("mtd: rawnand: qcom: convert driver to nand_scan()") Signed-off-by: Christian Lamparter <chunkeey@gmail.com> --- drivers/mtd/nand/raw/qcom_nandc.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)