linux-kernel - RE: [PATCH v2] scsi: core: Fix shost->cmd_per_lun calculation in scsi_add_host_with

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR21MB127049F258715B6F33D1B2B0BFB19@BYAPR21MB1270.namprd21.prod.outlook.com>
Date:   Thu, 7 Oct 2021 22:52:03 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     John Garry <john.garry@...wei.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "ming.lei@...hat.com" <ming.lei@...hat.com>,
        "bvanassche@....org" <bvanassche@....org>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        Long Li <longli@...rosoft.com>,
        Michael Kelley <mikelley@...rosoft.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v2] scsi: core: Fix shost->cmd_per_lun calculation in
 scsi_add_host_with_dma()

> From: John Garry <john.garry@...wei.com>
> Sent: Thursday, October 7, 2021 2:42 PM
> 
> On 07/10/2021 18:49, Dexuan Cui wrote:
> > After commit ea2f0f77538c, a 416-CPU VM running on Hyper-V hangs during
> > boot because scsi_add_host_with_dma() sets shost->cmd_per_lun to a
> > negative number (the below numbers may differ in different kernel versions):
> > in drivers/scsi/storvsc_drv.c, 	storvsc_drv_init() sets
> > 'max_outstanding_req_per_channel' to 352, and storvsc_probe() sets
> > 'max_sub_channels' to (416 - 1) / 4 = 103 and sets scsi_driver.can_queue to
> > 352 * (103 + 1) * (100 - 10) / 100 = 32947, which exceeds SHRT_MAX.
> 
> I think that you just need to mention that if can_queue exceeds
> SHRT_MAX, then there is a data truncation issue.

I just hoped the explanation how the too big 'can_queue' value is generated is helpful.

OK, I think I can change the commit log to:
 
After commit ea2f0f77538c, a 416-CPU VM running on Hyper-V hangs during
boot because the hv_storvsc driver sets scsi_driver.can_queue to an "int"
value that exceeds SHRT_MAX, and hence scsi_add_host_with_dma() sets
shost->cmd_per_lun to a negative "short" number. 

Use min_t(int, ...) to fix the issue.

> It looks ok, I'd just like to test it a bit more.
> 
> Thanks,
> John

Thanks! I'll post v3 with the above commit log, and I look forward to your review/test.

> > v2 directly fixes the scsi core change instead as Michael Kelley and
> > John Garry suggested (refer to the above link).
> 
> To be fair, it was Michael's suggestion
 
Yeah. Michael always gives good suggstions when reviewing patches. :-)