>>>>> "Hannes" == Hannes Reinecke <hare@xxxxxxx> writes:
Hannes> Hehe. It helps to actually test patches.
I did. However, with scsi_debug I have all sorts of sanity checks in
place. Not sure what your disk is connected to?
Hannes> sd 6:0:0:0: [sdb] Write Protect is off sd 6:0:0:0: [sdb] Mode
Hannes> Sense: cf 00 10 08 sd 6:0:0:0: [sdb] Write cache: disabled, read
Hannes> cache: enabled, supports DPO and FUA sd 6:0:0:0: [sdb] Enabling
Hannes> DIX T10-DIF-TYPE1-CRC protection sd 6:0:0:0: [sdb] Attached SCSI
Hannes> disk
DIX and DIF are orthogonal. If the HBA driver does not support the
appropriate DIF type we'll still enable DIX. That's a feature.
But there are two issues here. First we should not configure DIX if
capacity is 0. Secondly, we need some bounds checking in the host mask
checking to protect against crackpot disks.
Updated patch below...
sd: Ensure we correctly disable devices with unknown protection type
We set the capacity to zero when we discovered a device formatted with
an unknown DIF protection type. However, the read_capacity code would
override the capacity and cause the device to be enabled regardless.
Make sd_read_protection_type() return an error if the protection type is
unknown. Also prevent duplicate printk lines when the device is being
revalidated.
Reported-by: Hannes Reinecke <hare@xxxxxxx>
Signed-off-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 1cf2d5d..7a22db3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1820,34 +1820,42 @@ sd_spinup_disk(struct scsi_disk *sdkp)
/*
* Determine whether disk supports Data Integrity Field.
*/
-static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
+static int sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffer)
{
struct scsi_device *sdp = sdkp->device;
u8 type;
+ int ret = 0;
if (scsi_device_protection(sdp) == 0 || (buffer[12] & 1) == 0)
- return;
+ return ret;
type = ((buffer[12] >> 1) & 7) + 1; /* P_TYPE 0 = Type 1 */
- if (type == sdkp->protection_type || !sdkp->first_scan)
- return;
+ if (type > SD_DIF_TYPE3_PROTECTION)
+ ret = -ENODEV;
+ else if (scsi_host_dif_capable(sdp->host, type))
+ ret = 1;
+
+ if (sdkp->first_scan || type != sdkp->protection_type)
+ switch (ret) {
+ case -ENODEV:
+ sd_printk(KERN_ERR, sdkp, "formatted with unsupported" \
+ " protection type %u. Disabling disk!\n",
+ type);
+ break;
+ case 1:
+ sd_printk(KERN_NOTICE, sdkp,
+ "Enabling DIF Type %u protection\n", type);
+ break;
+ case 0:
+ sd_printk(KERN_NOTICE, sdkp,
+ "Disabling DIF Type %u protection\n", type);
+ break;
+ }
sdkp->protection_type = type;
- if (type > SD_DIF_TYPE3_PROTECTION) {
- sd_printk(KERN_ERR, sdkp, "formatted with unsupported " \
- "protection type %u. Disabling disk!\n", type);
- sdkp->capacity = 0;
- return;
- }
-
- if (scsi_host_dif_capable(sdp->host, type))
- sd_printk(KERN_NOTICE, sdkp,
- "Enabling DIF Type %u protection\n", type);
- else
- sd_printk(KERN_NOTICE, sdkp,
- "Disabling DIF Type %u protection\n", type);
+ return ret;
}
static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
@@ -1943,7 +1951,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
sector_size = get_unaligned_be32(&buffer[8]);
lba = get_unaligned_be64(&buffer[0]);
- sd_read_protection_type(sdkp, buffer);
+ if (sd_read_protection_type(sdkp, buffer) < 0) {
+ sdkp->capacity = 0;
+ return -ENODEV;
+ }
if ((sizeof(sdkp->capacity) == 4) && (lba >= 0xffffffffULL)) {
sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a "
@@ -2788,7 +2799,8 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
}
add_disk(gd);
- sd_dif_config_host(sdkp);
+ if (sdkp->capacity)
+ sd_dif_config_host(sdkp);
sd_revalidate_disk(gd);
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 5f7d5b3..b19223d 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -871,7 +871,11 @@ static inline unsigned int scsi_host_dif_capable(struct Scsi_Host *shost, unsign
static unsigned char cap[] = { 0,
SHOST_DIF_TYPE1_PROTECTION,
SHOST_DIF_TYPE2_PROTECTION,
- SHOST_DIF_TYPE3_PROTECTION };
+ SHOST_DIF_TYPE3_PROTECTION,
+ };
+
+ if (target_type > SHOST_DIF_TYPE3_PROTECTION)
+ return 0;
return shost->prot_capabilities & cap[target_type] ? target_type : 0;
}
@@ -884,6 +888,9 @@ static inline unsigned int scsi_host_dix_capable(struct Scsi_Host *shost, unsign
SHOST_DIX_TYPE2_PROTECTION,
SHOST_DIX_TYPE3_PROTECTION };
+ if (target_type > SHOST_DIX_TYPE3_PROTECTION)
+ return 0;
+
return shost->prot_capabilities & cap[target_type];
#endif
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
