linux-kernel - Re: [PATCH] scsi: isci: initialize shost fully before calling scsi_add

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]

Date:   Tue, 08 Jan 2019 16:25:26 -0500
From:   Jeff Moyer <jmoyer@...hat.com>
To:     Logan Gunthorpe <logang@...tatee.com>
Cc:     linux-scsi@...r.kernel.org, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Intel SCU Linux support <intel-linux-scu@...el.com>,
        Artur Paszkiewicz <artur.paszkiewicz@...el.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Christoph Hellwig <hch@....de>, Jens Axboe <axboe@...nel.dk>,
        james.smart@...adcom.com, dick.kennedy@...adcom.com
Subject: Re: [PATCH] scsi: isci: initialize shost fully before calling scsi_add_host()

Logan Gunthorpe <logang@...tatee.com> writes:

> scsi_mq_setup_tags(), which is called by scsi_add_host(), calculates
> the command size to allocate based on the prot_capabilities. In the
> isci driver, scsi_host_set_prot() is called after scsi_add_host()
> so the command size gets calculated to be smaller than it needs to be.
> Eventually, scsi_mq_init_request() locates the 'prot_sdb' after the command
> assuming it was sized correctly and a buffer overrun may occur.

[...]

> To prevent this, the calls to scsi_host_set_prot() are moved into
> isci_host_alloc() before the call to scsi_add_host(). Out of caution,
> also move the similar call to scsi_host_set_guard().
>
> Fixes: 3d2d75254915 ("[SCSI] isci: T10 DIF support")
> Link: http://lkml.kernel.org/r/da851333-eadd-163a-8c78-e1f4ec5ec857@deltatee.com
> Signed-off-by: Logan Gunthorpe <logang@...tatee.com>
> Cc: Intel SCU Linux support <intel-linux-scu@...el.com>
> Cc: Artur Paszkiewicz <artur.paszkiewicz@...el.com>
> Cc: "James E.J. Bottomley" <jejb@...ux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@...cle.com>
> Cc: Christoph Hellwig <hch@....de>
> Cc: Jens Axboe <axboe@...nel.dk>
> Cc: Jeff Moyer <jmoyer@...hat.com>

Nice job, and excellent commit message.  We'll need a similar patch for
lpfc.

Reviewed-by: Jeff Moyer <jmoyer@...hat.com>

> ---
>  drivers/scsi/isci/init.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
> index 68b90c4f79a3..1727d0c71b12 100644
> --- a/drivers/scsi/isci/init.c
> +++ b/drivers/scsi/isci/init.c
> @@ -576,6 +576,13 @@ static struct isci_host *isci_host_alloc(struct pci_dev *pdev, int id)
>  	shost->max_lun = ~0;
>  	shost->max_cmd_len = MAX_COMMAND_SIZE;
>  
> +	/* turn on DIF support */
> +	scsi_host_set_prot(shost,
> +			   SHOST_DIF_TYPE1_PROTECTION |
> +			   SHOST_DIF_TYPE2_PROTECTION |
> +			   SHOST_DIF_TYPE3_PROTECTION);
> +	scsi_host_set_guard(shost, SHOST_DIX_GUARD_CRC);
> +
>  	err = scsi_add_host(shost, &pdev->dev);
>  	if (err)
>  		goto err_shost;
> @@ -663,13 +670,6 @@ static int isci_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  			goto err_host_alloc;
>  		}
>  		pci_info->hosts[i] = h;
> -
> -		/* turn on DIF support */
> -		scsi_host_set_prot(to_shost(h),
> -				   SHOST_DIF_TYPE1_PROTECTION |
> -				   SHOST_DIF_TYPE2_PROTECTION |
> -				   SHOST_DIF_TYPE3_PROTECTION);
> -		scsi_host_set_guard(to_shost(h), SHOST_DIX_GUARD_CRC);
>  	}
>  
>  	err = isci_setup_interrupts(pdev);