Re: [PATCH] scsi: core: fix uninit-value access of variable sshdr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/10/19 5:05 AM, zhengbin wrote:
kmsan report a warning in 5.1-rc4:

BUG: KMSAN: uninit-value in sr_get_events drivers/scsi/sr.c:207 [inline]
BUG: KMSAN: uninit-value in sr_check_events+0x2cf/0x1090 drivers/scsi/sr.c:243
CPU: 1 PID: 13858 Comm: syz-executor.0 Tainted: G    B             5.1.0-rc4+ #8
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x173/0x1d0 lib/dump_stack.c:113
  kmsan_report+0x131/0x2a0 mm/kmsan/kmsan.c:619
  __msan_warning+0x7a/0xf0 mm/kmsan/kmsan_instr.c:310
  sr_get_events drivers/scsi/sr.c:207 [inline]
  sr_check_events+0x2cf/0x1090 drivers/scsi/sr.c:243

The reason is as follows:
sr_get_events
   struct scsi_sense_hdr sshdr;  -->uninit
   scsi_execute_req              -->If fail, will not set sshdr
   scsi_sense_valid(&sshdr)      -->access sshdr

We can init sshdr in sr_get_events, but there have many callers of
scsi_execute, scsi_execute_req, we have to troubleshoot all callers,
the simpler way is init sshdr in __scsi_execute.

BTW: we can't just init sshdr->response_code, sr_do_ioctl use
sshdr->sense_key(Need to troubleshoot all callers)

Signed-off-by: zhengbin <zhengbin13@xxxxxxxxxx>
---
  drivers/scsi/scsi_lib.c | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5447738..037fb2a 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -255,6 +255,12 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
  	struct scsi_request *rq;
  	int ret = DRIVER_ERROR << 24;

+	/*
+	 * need to initial sshdr to avoid uninit-value access
+	 */
+	if (sshdr)
+		memset(sshdr, 0, sizeof(struct scsi_sense_hdr));
+
  	req = blk_get_request(sdev->request_queue,
  			data_direction == DMA_TO_DEVICE ?
  			REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, BLK_MQ_REQ_PREEMPT);

From SAM-5: "Sense data shall be made available by the logical unit if a command terminates with CHECK CONDITION status or other conditions (e.g., the processing of a REQUEST SENSE command). The format, content, and conditions under which sense data shall be prepared by the logical unit are as defined in this standard, SPC-4, the applicable command standard, and the applicable SCSI transport protocol standard."

Apparently sr_check_events() submits a GET EVENT STATUS NOTIFICATION command and it even evaluates the sense data if that command succeeded. I'm not aware of other scsi_execute() callers that do this. So I'm not sure that modifying __scsi_execute() is the best approach. I'm wondering whether the sr driver should be fixed instead of modifying __scsi_execute().

Thanks,

Bart.



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux