Re: [PATCH 5.10 264/530] spi: spi-zynqmp-gqspi: fix use-after-free in zynqmp_qspi_exec_op

Hi!

When handling op->addr, it is using the buffer "tmpbuf" which has been freed. This will trigger a use-after-free KASAN warning. Let's use temporary variables to store op->addr.val and op->cmd.opcode to fix this issue.

I believe this is "cure worse than a disassease".

+++ b/drivers/spi/spi-zynqmp-gqspi.c @@ -926,8 +926,9 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem, struct zynqmp_qspi *xqspi = spi_controller_get_devdata (mem->spi->master); int err = 0, i;

• u8 *tmpbuf; u32 genfifoentry = 0;

• u16 opcode = op->cmd.opcode;
• u64 opaddr;

dev_dbg(xqspi->dev, "cmd:%#x mode:%d.%d.%d.%d\n", op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth, @@ -940,14 +941,8 @@ static int zynqmp_qspi_exec_op(struct spi_mem *mem, genfifoentry |= xqspi->genfifobus; if (op->cmd.opcode) {

• tmpbuf = kzalloc(op->cmd.nbytes, GFP_KERNEL | GFP_DMA);
  

• if (!tmpbuf) {
  

• 	mutex_unlock(&xqspi->op_lock);
  

• 	return -ENOMEM;
  

• }
  

• tmpbuf[0] = op->cmd.opcode;
  

  reinit_completion(&xqspi->data_completion);

• xqspi->txbuf = tmpbuf;
  

• xqspi->txbuf = &opcode;
  

  xqspi->rxbuf = NULL; xqspi->bytes_to_transfer = op->cmd.nbytes; xqspi->bytes_to_receive = 0;

So this replaces "op->cmd.nbytes" bytes big DMA buffer with 2 bytes on stack.

First, if op->cmd.nbytes is > 2, DMA will overrun that buffer. That can't be healthy.

Second, you really should not run DMA from on-stack buffers.

Best regards, Pavel