linux-kernel - Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME

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: <20140711063216.GA20660@infradead.org>
Date:	Thu, 10 Jul 2014 23:32:16 -0700
From:	"hch@...radead.org" <hch@...radead.org>
To:	James Bottomley <jbottomley@...allels.com>
Cc:	"kys@...rosoft.com" <kys@...rosoft.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"mkp@....net" <mkp@....net>,
	"hch@...radead.org" <hch@...radead.org>,
	"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
	"apw@...onical.com" <apw@...onical.com>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"ohering@...e.com" <ohering@...e.com>,
	"jasowang@...hat.com" <jasowang@...hat.com>
Subject: Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16

On Wed, Jul 09, 2014 at 10:27:24PM +0000, James Bottomley wrote:
> If we fix it at source, why would there be any need to filter?  That's
> the reason the no_write_same flag was introduced.  If we can find and
> fix the bug, it can go back into the stable trees as a bug fix, hence
> nothing should ever emit write_same(10 or 16) and additional driver code
> is redundant (and counter productive, since if this ever breaks again
> you're our best canary).
> 
> This looks like it might be the problem but Martin should confirm (I
> think the problem comes to us from the RC16 code which unconditionally
> sets WS16).

I think the problem is a differnet one.  If we have the logical
provisioning EVPD it configures what method to use, but if we don't have
one we simply check for a max unmap blocks field, and if that's not
present use WRITE SAME.

The patch checks the no_write_same flag before doing that, for which
we also have to do the write_same setup before the discard setup
in sd_revalidate_disk.

Ky: does hyperv support UNMAP?  If so any idea why it doesn't set
the maximum unmap block count field in the EVPD?

If we want to enable UNMAP in this case I'd prefer a blacklist entry
than trying UNMAP despite the device not advertising it.

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index ba756b1..fbccfd2 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2614,9 +2614,10 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
 
 			if (sdkp->max_unmap_blocks)
 				sd_config_discard(sdkp, SD_LBP_UNMAP);
-			else
+			else if (!sdkp->device->no_write_same)
 				sd_config_discard(sdkp, SD_LBP_WS16);
-
+			else
+				sd_config_discard(sdkp, SD_LBP_DISABLE);
 		} else {	/* LBP VPD page tells us what to use */
 
 			if (sdkp->lbpu && sdkp->max_unmap_blocks)
@@ -2766,6 +2767,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	 */
 	if (sdkp->media_present) {
 		sd_read_capacity(sdkp, buffer);
+		sd_read_write_same(sdkp, buffer);
 
 		if (sd_try_extended_inquiry(sdp)) {
 			sd_read_block_provisioning(sdkp);
@@ -2776,7 +2778,6 @@ static int sd_revalidate_disk(struct gendisk *disk)
 		sd_read_write_protect_flag(sdkp, buffer);
 		sd_read_cache_type(sdkp, buffer);
 		sd_read_app_tag_own(sdkp, buffer);
-		sd_read_write_same(sdkp, buffer);
 	}
 
 	sdkp->first_scan = 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/