Kernel crash with unsupported DIF protection type
Hi all,
I recently got my hands on some weird drives, insisting on having been formatted with protection type 7:
# sg_readcap --16 /dev/sdb Read Capacity results: Protection: prot_en=1, p_type=6, p_i_exponent=0 Logical block provisioning: lbpme=0, lbprz=0 Last logical block address=1758174767 (0x68cb9e2f), Number of logical blocks=1758174768 Logical block length=512 bytes Logical blocks per physical block exponent=0 Lowest aligned logical block address=0 Hence: Device size: 900185481216 bytes, 858483.8 MiB, 900.19 GB
(I know. I've already complained.)
However, this drive causes a horrible crash:
sd 0:0:1:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838 GiB)
sd 0:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
sd 0:0:1:0: [sdb] Sense Key : Medium Error [current] sd 0:0:1:0: [sdb] Add. Sense: Medium format corrupted sd 0:0:1:0: [sdb] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00end_request: I/O error, dev sdb, sector 0
Buffer I/O error on device sdb, logical block 0
[ tons and tons of I/O errors ]
[ 15.551689] sd 0:0:1:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC protection [ 15.561353] ------------[ cut here ]------------ [ 15.569416] kernel BUG at
/usr/src/linux-3.0/drivers/scsi/scsi_lib.c:1069!
There are several odd things happening here:
- It says 'Disabling disk', which _should_ have set the capacity to '0':
drivers/scsi/sd.c:sd_read_protection_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;
}- it enables type 1 protection, which it evidently is not.
I've attached a tentative patch, which allows the system to boot. However, I'm not completely happy with that, as the capacity is _still_ updated after revalidation:
sd 0:0:1:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
sd 0:0:1:0: [sdb] 0 512-byte logical blocks: (0 B/0 B)
sd 0:0:1:0: [sdb] Write Protect is off
sd 0:0:1:0: [sdb] Mode Sense: cf 00 10 08
sd 0:0:1:0: [sdb] Write cache: disabled, read cache: enabled, supports DPO and FUA
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838 GiB) sd 0:0:1:0: [sdb] Attached SCSI disk
Thoughts?
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
Quoted 1 line
From
88f73f9e3c9e9aaa6af73885db9c6230edab6fe0Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <[email protected]>
Date: Tue, 18 Sep 2012 10:37:52 +0200
Subject: [PATCH] sd: disable disk with incorrect protection information
SPC-3 only defines DIF protection up to type 3. Other types are invalid and the drive cannot be used. So make sure to disable the drive correctly by setting the capacity to 0.
drivers/scsi/sd.c +11 -8
@@ -1671,26 +1671,25 @@ 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;if (scsi_device_protection (sdp) ==0 || (buffer[12 ] &1 ) ==0 ) -return ; +return 0 ; type = ((buffer[12 ] >>1 ) &7 ) +1 ;/* P_TYPE 0 = Type 1 */ if (type == sdkp->protection_type || !sdkp->first_scan) -return ; +return 0 ; 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 ; +return -EINVAL ; }if (scsi_host_dif_capable (sdp->host, type))
@@ -1699,6 +1698,7 @@ static void sd_read_protection_type(struct scsi_disk *sdkp, unsigned char *buffeelse sd_printk (KERN_NOTICE , sdkp,"Disabling DIF Type %u protection\n" , type); +return 0 ; } static void read_capacity_error(struct scsi_disk *sdkp, struct scsi_device *sdp,
@@ -1794,8 +1794,6 @@ 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 ((sizeof (sdkp->capacity) ==4 ) && (lba >=0xffffffffULL )) {sd_printk (KERN_ERR , sdkp,"Too big for this kernel. Use a " "kernel compiled with support for large block "
@@ -1804,6 +1802,12 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,return -EOVERFLOW ; } +if (sd_read_protection_type (sdkp, buffer)) +/* Invalid or unhandled protection type */ + sdkp->capacity =0 ; +else + sdkp->capacity = lba +1 ; +/* Logical blocks per physical block exponent */ sdkp->physical_block_size = (1 << (buffer[13 ] &0xf )) * sector_size;
@@ -1823,7 +1827,6 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,sd_config_discard (sdkp,SD_LBP_WS16 ); } - sdkp->capacity = lba +1 ;return sector_size; }
drivers/scsi/sd_dif.c +1 -1
@@ -320,7 +320,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp) dif =0 ; dix =1 ; } -if (!dix) +if (!dix || type >SD_DIF_TYPE3_PROTECTION )return ;/* Enable DMA of protection information */
Quoted 68 lines · Hannes Reinecke
Hi all,
I recently got my hands on some weird drives, insisting on having been formatted with protection type 7:
# sg_readcap --16 /dev/sdb Read Capacity results: Protection: prot_en=1, p_type=6, p_i_exponent=0 Logical block provisioning: lbpme=0, lbprz=0 Last logical block address=1758174767 (0x68cb9e2f), Number of logical blocks=1758174768 Logical block length=512 bytes Logical blocks per physical block exponent=0 Lowest aligned logical block address=0 Hence: Device size: 900185481216 bytes, 858483.8 MiB, 900.19 GB(I know. I've already complained.)
However, this drive causes a horrible crash:sd 0:0:1:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838 GiB)
sd 0:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
sd 0:0:1:0: [sdb] Sense Key : Medium Error [current] sd 0:0:1:0: [sdb] Add. Sense: Medium format corrupted sd 0:0:1:0: [sdb] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00end_request: I/O error, dev sdb, sector 0
Buffer I/O error on device sdb, logical block 0[ tons and tons of I/O errors ]
[ 15.551689] sd 0:0:1:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC protection [ 15.561353] ------------[ cut here ]------------ [ 15.569416] kernel BUG at
/usr/src/linux-3.0/drivers/scsi/scsi_lib.c:1069!There are several odd things happening here:
- It says 'Disabling disk', which _should_ have set the capacity to '0':drivers/scsi/sd.c:sd_read_protection_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; }- it enables type 1 protection, which it evidently is not.
I've attached a tentative patch, which allows the system to boot. However, I'm not completely happy with that, as the capacity is _still_ updated after revalidation:
sd 0:0:1:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
sd 0:0:1:0: [sdb] 0 512-byte logical blocks: (0 B/0 B)
sd 0:0:1:0: [sdb] Write Protect is off
sd 0:0:1:0: [sdb] Mode Sense: cf 00 10 08
sd 0:0:1:0: [sdb] Write cache: disabled, read cache: enabled, supports DPO and FUA
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838 GiB) sd 0:0:1:0: [sdb] Attached SCSI disk
Quoted 1 line
Thoughts?
The "Medium format corrupted" additional sense qualifier occurs (with Seagate disks) when a FORMAT UNIT command is interrupted. So maybe, for good measure, that disk also sets the DIF protection type to an unsupported value (i.e. 7). So re-doing a FORMAT UNIT may clear that state.
Obviously the kernel should not crash when faced with such a disk.
Doug Gilbert
Quoted 70 lines · Douglas Gilbert
Quoted 69 lines · Hannes Reinecke
Hi all,
I recently got my hands on some weird drives, insisting on having been formatted with protection type 7:
# sg_readcap --16 /dev/sdb Read Capacity results: Protection: prot_en=1, p_type=6, p_i_exponent=0 Logical block provisioning: lbpme=0, lbprz=0 Last logical block address=1758174767 (0x68cb9e2f), Number of logical blocks=1758174768 Logical block length=512 bytes Logical blocks per physical block exponent=0 Lowest aligned logical block address=0 Hence: Device size: 900185481216 bytes, 858483.8 MiB, 900.19 GB(I know. I've already complained.)
However, this drive causes a horrible crash:sd 0:0:1:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838 GiB)
sd 0:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
sd 0:0:1:0: [sdb] Sense Key : Medium Error [current] sd 0:0:1:0: [sdb] Add. Sense: Medium format corrupted sd 0:0:1:0: [sdb] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00end_request: I/O error, dev sdb, sector 0
Buffer I/O error on device sdb, logical block 0[ tons and tons of I/O errors ]
[ 15.551689] sd 0:0:1:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC protection [ 15.561353] ------------[ cut here ]------------ [ 15.569416] kernel BUG at
/usr/src/linux-3.0/drivers/scsi/scsi_lib.c:1069!There are several odd things happening here:
- It says 'Disabling disk', which _should_ have set the capacity to '0':drivers/scsi/sd.c:sd_read_protection_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; }- it enables type 1 protection, which it evidently is not.
I've attached a tentative patch, which allows the system to boot. However, I'm not completely happy with that, as the capacity is _still_ updated after revalidation:
sd 0:0:1:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
sd 0:0:1:0: [sdb] 0 512-byte logical blocks: (0 B/0 B) sd 0:0:1:0: [sdb] Write Protect is off
sd 0:0:1:0: [sdb] Mode Sense: cf 00 10 08
sd 0:0:1:0: [sdb] Write cache: disabled, read cache: enabled, supports DPO and FUA
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838 GiB)
sd 0:0:1:0: [sdb] Attached SCSI disk
Quoted 8 lines
Quoted 1 line
Thoughts?
The "Medium format corrupted" additional sense qualifier occurs (with Seagate disks) when a FORMAT UNIT command is interrupted. So maybe, for good measure, that disk also sets the DIF protection type to an unsupported value (i.e. 7). So re-doing a FORMAT UNIT may clear that state.
Yeah, that's what I'm doing now.
Incidentally:
sg_format manpage says:
Format with type 1 protection:
sg_format --format --fmtpinfo=3 --pfu /dev/sdm
but the '--pfu' option requires an argument.
According to sbc3r23 the command should read:
sg_format --format --fmtpinfo=3 --pfu=1 /dev/sdm
Correct?
Quoted 3 lines
Obviously the kernel should not crash when faced with such a disk.
Precisely.
Especially the 'Disabling disk' no-op is worrying.
Cheers,
Hannes
Signature · 10 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 87 lines · Hannes Reinecke
Quoted 70 lines · Douglas Gilbert
Quoted 69 lines · Hannes Reinecke
Hi all,
I recently got my hands on some weird drives, insisting on having been formatted with protection type 7:
# sg_readcap --16 /dev/sdb Read Capacity results: Protection: prot_en=1, p_type=6, p_i_exponent=0 Logical block provisioning: lbpme=0, lbprz=0 Last logical block address=1758174767 (0x68cb9e2f), Number of logical blocks=1758174768 Logical block length=512 bytes Logical blocks per physical block exponent=0 Lowest aligned logical block address=0 Hence: Device size: 900185481216 bytes, 858483.8 MiB, 900.19 GB(I know. I've already complained.)
However, this drive causes a horrible crash:sd 0:0:1:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838 GiB)
sd 0:0:1:0: [sdb] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
sd 0:0:1:0: [sdb] Sense Key : Medium Error [current] sd 0:0:1:0: [sdb] Add. Sense: Medium format corrupted sd 0:0:1:0: [sdb] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00end_request: I/O error, dev sdb, sector 0
Buffer I/O error on device sdb, logical block 0[ tons and tons of I/O errors ]
[ 15.551689] sd 0:0:1:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC protection [ 15.561353] ------------[ cut here ]------------ [ 15.569416] kernel BUG at
/usr/src/linux-3.0/drivers/scsi/scsi_lib.c:1069!There are several odd things happening here:
- It says 'Disabling disk', which _should_ have set the capacity to '0':drivers/scsi/sd.c:sd_read_protection_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; }- it enables type 1 protection, which it evidently is not.
I've attached a tentative patch, which allows the system to boot. However, I'm not completely happy with that, as the capacity is _still_ updated after revalidation:
sd 0:0:1:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
sd 0:0:1:0: [sdb] 0 512-byte logical blocks: (0 B/0 B) sd 0:0:1:0: [sdb] Write Protect is off
sd 0:0:1:0: [sdb] Mode Sense: cf 00 10 08
sd 0:0:1:0: [sdb] Write cache: disabled, read cache: enabled, supports DPO and FUA
sd 0:0:1:0: [sdb] 1758174768 512-byte logical blocks: (900 GB/838 GiB)
sd 0:0:1:0: [sdb] Attached SCSI diskQuoted 8 lines
Quoted 1 line
Thoughts?
The "Medium format corrupted" additional sense qualifier occurs (with Seagate disks) when a FORMAT UNIT command is interrupted. So maybe, for good measure, that disk also sets the DIF protection type to an unsupported value (i.e. 7). So re-doing a FORMAT UNIT may clear that state.
Yeah, that's what I'm doing now.
Incidentally:
sg_formatmanpage says:Format with type 1 protection:
Quoted 8 lines
sg_format--format --fmtpinfo=3 --pfu/dev/sdmbut the '--pfu' option requires an argument.
According to sbc3r23 the command should read:
sg_format--format --fmtpinfo=3 --pfu=1/dev/sdmCorrect?
Yes. And it is corrected in the 1.34 betas that I have sent you :-)
I placed a copy of my latest sg3_utils-1.34 beta
in the News section at the top of this page:
http://sg.danny.cz/sg
Doug Gilbert
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Hannes" == Hannes Reinecke <[email protected]> writes:
Hannes> I recently got my hands on some weird drives, insisting on Hannes> having been formatted with protection type 7:
Lovely :|
Hannes> I've attached a tentative patch, which allows the system to Hannes> boot. However, I'm not completely happy with that, as the Hannes> capacity is _still_ updated after revalidation:
How about this?
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.
drivers/scsi/sd.c +29 -18
@@ -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 "
Hannes Reinecke
Acked-byQuoted 7 lines · Martin K. Petersen
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Hannes" == Hannes Reinecke <[email protected]> writes:
Hannes> I recently got my hands on some weird drives, insisting on Hannes> having been formatted with protection type 7:
Lovely :|
Hehe. Probably _really_ future drives :-)
Quoted 88 lines
Hannes> I've attached a tentative patch, which allows the system to Hannes> boot. However, I'm not completely happy with that, as the Hannes> capacity is _still_ updated after revalidation:
How about this?
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_capacitycode 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 <[email protected]>Signed-off-by Martin K. Petersen <[email protected]>drivers/scsi/sd.c +25
-17@@ -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 ]);
drivers/scsi/sd.c static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, · line 1960
Quoted 10 lines
drivers/scsi/sd.c +4
-1lba =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 "
Yup. That's what I had in mind.
Cheers,
Hannes
Signature · 10 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 94 lines · Martin K. Petersen
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Hannes" == Hannes Reinecke <[email protected]> writes:
Hannes> I recently got my hands on some weird drives, insisting on Hannes> having been formatted with protection type 7:
Lovely :|
Hannes> I've attached a tentative patch, which allows the system to Hannes> boot. However, I'm not completely happy with that, as the Hannes> capacity is _still_ updated after revalidation:
How about this?
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_capacitycode 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 <[email protected]>Signed-off-by Martin K. Petersen <[email protected]>drivers/scsi/sd.c +25
-17@@ -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 ]);
drivers/scsi/sd.c static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, · line 1960
Quoted 10 lines
drivers/scsi/sd.c +4
-1lba =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 "
Hehe. It helps to actually test patches.
Now I get:
sd 6:0:0:0: [sdb] formatted with unsupported protection type 7. Disabling disk!
and silence after that.
However, I still get:
sd 6:0:0:0: [sdb] Write Protect is off
sd 6:0:0:0: [sdb] Mode Sense: cf 00 10 08
sd 6:0:0:0: [sdb] Write cache: disabled, read cache: enabled, supports DPO and FUA
sd 6:0:0:0: [sdb] Enabling DIX T10-DIF-TYPE1-CRC protection sd 6:0:0:0: [sdb] Attached SCSI disk
which is a bit odd, given that the disk is disabled. Plus that we never actually claimed to support type1. So maybe we should add this patch, too?
drivers/scsi/sd_dif.c +1 -1
@@ -320,7 +320,7 @@ void sd_dif_config_host(struct scsi_disk *sdkp) dif =0 ; dix =1 ; } -if (!dix) +if (!dix || type >SD_DIF_TYPE3_PROTECTION )return ;/* Enable DMA of protection information */
But then, main issue is resolved, as the system continues to boot. So my Acked-by: still stands.
Cheers,
Hannes
Signature · 10 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Hannes" == Hannes Reinecke <[email protected]> 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.
drivers/scsi/sd.c +31 -19
@@ -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);
include/scsi/scsi_host.h +8 -1
@@ -871,7 +871,11 @@ static inline unsigned int scsi_host_dif_capable(struct Scsi_Host *shost, unsignstatic 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, unsignSHOST_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 ;
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Martin" == Martin K Petersen <[email protected]> writes:
Martin> Updated patch below...
This time without mangled closing bracket...
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.
drivers/scsi/sd.c +31 -19
@@ -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);
include/scsi/scsi_host.h +6 -0
@@ -873,6 +873,9 @@ static inline unsigned int scsi_host_dif_capable(struct Scsi_Host *shost, unsignSHOST_DIF_TYPE2_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 +887,9 @@ static inline unsigned int scsi_host_dix_capable(struct Scsi_Host *shost, unsignSHOST_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 ;