mtd: fsl-quadspi: kernel panic in fsl_qspi_nor_write() due to access across page boundary

mtd: fsl-quadspi: kernel panic in fsl_qspi_nor_write() due to access across page boundary

[Sebastian Falbesoner]

Hi,

we noticed that whenever we copy large files (several MB) within a JFFS2
partition for a flash memory using the fsl-quadspi driver, sporadic kernel
panics happen, e.g.:

--------------------------------------------------------------------------------
Unable to handle kernel paging request at virtual address 7fe54000
pgd = 88ce4000
[7fe54000] *pgd=9fd5b811, *pte=00000000, *ppte=00000000
Internal error: Oops: 7 1 PREEMPT SMP ARM
Modules linked in: [...]
CPU: 0 PID: 621 Comm: _SVM Tainted: G O 4.1.15-1.2.0+g77f6154 #1
Hardware name: Freescale i.MX7 Dual (Device Tree)
task: 88468000 ti: 88d60000 task.ti: 88d60000
PC is at fsl_qspi_nor_write+0x74/0x13c
LR is at fsl_qspi_write+0x34/0xa8
[...]
[<80362314>] (fsl_qspi_nor_write) from [<80362410>] (fsl_qspi_write+0x34/0xa8)
[<80362410>] (fsl_qspi_write) from [<80360c04>] (spi_nor_write+0x134/0x178)
[<80360c04>] (spi_nor_write) from [<80358c78>] (mtd_writev+0xa4/0xec)
[<80358c78>] (mtd_writev) from [<8020b5d0>] (jffs2_flash_writev+0x3f0/0x480)
[<8020b5d0>] (jffs2_flash_writev) from [<802047b8>]
(jffs2_write_dnode+0xc8/0x320)
[<802047b8>] (jffs2_write_dnode) from [<80204f74>]
(jffs2_write_inode_range+0x21c/0x2c0)
[<80204f74>] (jffs2_write_inode_range) from [<801ffc8c>]
(jffs2_write_end+0xe0/0x1fc)
[<801ffc8c>] (jffs2_write_end) from [<800a3ec0>]
(generic_perform_write+0x114/0x1ac)
[<800a3ec0>] (generic_perform_write) from [<800a58bc>]
(__generic_file_write_iter+0x178/0x1c8)
[<800a58bc>] (__generic_file_write_iter) from [<800a5a48>]
(generic_file_write_iter+0x13c/0x258)
[<800a5a48>] (generic_file_write_iter) from [<800e05dc>] (__vfs_write+0xa8/0xd8)
[<800e05dc>] (__vfs_write) from [<800e0d84>] (vfs_write+0x90/0x16c)
[<800e0d84>] (vfs_write) from [<800e159c>] (SyS_write+0x44/0x9c)
[<800e159c>] (SyS_write) from [<8000f400>] (ret_fast_syscall+0x0/0x3c)
[...]
--------------------------------------------------------------------------------

After analyzing the backtrace of several panic incidents we came to the
following conclusions:

- the panic happens in drivers/mtd/spi-nor/fsl-quadspi.c:fsl_qspi_nor_write() in
the following loop filling the TX data into the FIFO in 4-byte-chunks, when
the txbuf pointer is dereferenced:

static int fsl_qspi_nor_write(struct fsl_qspi *q, struct spi_nor *nor,
                u8 opcode, unsigned int to, u32 *txbuf,
                unsigned count, size_t *retlen)
{
    int ret, i, j;
    u32 tmp;
    ...
    /* fill the TX data to the FIFO */
    for (j = 0, i = ((count + 3) / 4); j < i; j++) {
        tmp = fsl_qspi_endian_xchg(q, *txbuf); // <<<<< BOOM!
        qspi_writel(q, tmp, q->iobase + QUADSPI_TBDR);
        txbuf++;
    }
    ...
}

- the virtual address where the access causes trouble is always in the form
  .....000, i.e. at the beginning of a page
- the parameters "txbuf" and "count" are set such that the very last byte has
  the virtual address in the form .....fff, i.e. at the end of a page

Obviously the bad case happens whenever we have the TX data pointer is unaligned
and the data is at the end of a page -- this causes the last iteration of the
filling loop to have an access across page boundaries, for example:

       txbuf (count=2)
         |
         v
...-+---+---+---+ || +---+---+---+-...
... |   | X | X | || | X | X |   | ...
...-+---+---+---+ || +---+---+---+-...
                  ^^
                   \\ page boundary

The one iteration would dereference the txbuf pointer, which accesses two bytes
within the valid page, but another bytes in the next page which we are not
guaranteed to have access to, which in turn can cause the kernel panic.

The obvious solution would be to guarantee that txbuf is aligned, but since
the calling function of fsl_qspi_nor_write() simply casts an incoming u_char
pointer to an u32 pointer, it is not obvious for me where to fix this:

static void fsl_qspi_write(struct spi_nor *nor, loff_t to,
        size_t len, size_t *retlen, const u_char *buf)
{
    struct fsl_qspi *q = nor->priv;

    fsl_qspi_nor_write(q, nor, nor->program_opcode, to,
                (u32 *)buf, len, retlen);
    ..... //    ^^^ this cast creates and unaligned memory pointer
}

Any recommendation on how to fix this problem?
Isn't unaligned memory access generally considered a bad practice in linux
kernel (according to Documentation/unaligned-memory-access.txt) and should be
avoided?

Best regards,
Sebastian Falbesoner

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/