linux-kernel - ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi")

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 09 Jun 2020 20:00:35 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     brking@...ibm.com, Christoph Hellwig <hch@....de>,
        Jens Axboe <axboe@...nel.dk>, linux-scsi@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        linuxppc-dev@...ts.ozlabs.org, linux-block@...r.kernel.org,
        linux-ide@...r.kernel.org
Subject: ipr crashes due to NULL dma_need_drain since cc97923a5bcc ("block: move dma drain handling to scsi")

Hi all,

I'm seeing crashes on powerpc with the ipr driver, which I'm fairly sure
are due to dma_need_drain being NULL.

The backtrace is:

  scsi_init_io+0x1d8/0x350
  scsi_queue_rq+0x7a4/0xc30
  blk_mq_dispatch_rq_list+0x1b0/0x910
  blk_mq_sched_dispatch_requests+0x154/0x270
  __blk_mq_run_hw_queue+0xa0/0x160
  __blk_mq_delay_run_hw_queue+0x244/0x250
  blk_mq_sched_insert_request+0x13c/0x250
  blk_execute_rq_nowait+0x88/0xb0
  blk_execute_rq+0x5c/0xf0
  __scsi_execute+0x10c/0x270
  scsi_mode_sense+0x144/0x440
  sr_probe+0x2e8/0x810
  really_probe+0x12c/0x580
  driver_probe_device+0x88/0x170
  device_driver_attach+0x11c/0x130
  __driver_attach+0xac/0x190
  bus_for_each_dev+0xa8/0x130
  driver_attach+0x34/0x50
  bus_add_driver+0x170/0x2b0
  driver_register+0xb4/0x1c0
  scsi_register_driver+0x2c/0x40
  init_sr+0x4c/0x80
  do_one_initcall+0x60/0x2b0
  kernel_init_freeable+0x2e0/0x3a0
  kernel_init+0x2c/0x148
  ret_from_kernel_thread+0x5c/0x74

And looking at the disassembly I think it's coming from:

static inline bool scsi_cmd_needs_dma_drain(struct scsi_device *sdev,
		struct request *rq)
{
	return sdev->dma_drain_len && blk_rq_is_passthrough(rq) &&
	       !op_is_write(req_op(rq)) &&
	       sdev->host->hostt->dma_need_drain(rq);
                                  ^^^^^^^^^^^^^^
}

Bisect agrees:

# first bad commit: [cc97923a5bccc776851c242b61015faf288d5c22] block: move dma drain handling to scsi


And looking at ipr.c, it constructs its scsi_host_template manually,
without using any of the macros that end up calling __ATA_BASE_SHT,
which populates dma_need_drain.

The obvious fix below works, the system boots and seems to be operating
normally, but I don't know enough (anything) about SCSI to say if it's
actually the correct fix.

cheers


diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index 7d77997d26d4..7d86f4ca266c 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -6731,6 +6731,7 @@ static struct scsi_host_template driver_template = {
 	.compat_ioctl = ipr_ioctl,
 #endif
 	.queuecommand = ipr_queuecommand,
+	.dma_need_drain = ata_scsi_dma_need_drain,
 	.eh_abort_handler = ipr_eh_abort,
 	.eh_device_reset_handler = ipr_eh_dev_reset,
 	.eh_host_reset_handler = ipr_eh_host_reset,