| Message ID | alpine.DEB.2.21.2104141306130.44318@angie.orcam.me.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bring the BusLogic host bus adapter driver up to Y2021 | expand |
On 14 Apr 2021, Maciej W. Rozycki stated: > Set the allocation length to 255 for the ATA Information VPD page > requested in the WRITE SAME handler, so as not to limit information > examined by `scsi_get_vpd_page' in the supported vital product data > pages unnecessarily. > > Originally it was thought that Areca hardware may have issues with a > valid allocation length supplied for a VPD inquiry, however older SCSI > standard revisions[1] consider 255 the maximum length allowed and what Aaaah. That explains a lot! (Not that I can remember what SCSI standard rev that Areca firmware claimed to implement. I know I never updated the firmware, so it's going to be something no newer than mid-2009 and probably quite a bit older.) > Nix, > > I can see you're still around. Would you therefore please be so kind > as to verify this change with your Areca hardware if you still have it? It's been up in the loft for years, but I'll get it out this weekend and give it a spin :) this'll let me make sure the disks still spin as well, which matters for an in-case-of-lightning-strike disaster-recovery backup box. (I just hope this kernel boots on it at all. It's about three years since I retired it... let's see!) > It looks to me like you were thinking in the right direction with: > <https://lore.kernel.org/linux-scsi/87vc3nuipg.fsf@spindle.srvr.nix/>. It's the sort of mistake I could see myself making: an easy mistake to make when so many things in C require buffer size - 1 or you get a disastrous security hole...
On Thu, 15 Apr 2021, Nix wrote: > > Set the allocation length to 255 for the ATA Information VPD page > > requested in the WRITE SAME handler, so as not to limit information > > examined by `scsi_get_vpd_page' in the supported vital product data > > pages unnecessarily. > > > > Originally it was thought that Areca hardware may have issues with a > > valid allocation length supplied for a VPD inquiry, however older SCSI > > standard revisions[1] consider 255 the maximum length allowed and what > > Aaaah. That explains a lot! (Not that I can remember what SCSI standard > rev that Areca firmware claimed to implement. I know I never updated the > firmware, so it's going to be something no newer than mid-2009 and > probably quite a bit older.) From the original discussion I gather Areca sometimes acts as a pass-through device to actual storage hardware, so it may well have been decided for the firmware to take a conservative approach and interpret the low order byte only. A genuine bug cannot be ruled out either of course, which I why I will appreciate your testing. > > I can see you're still around. Would you therefore please be so kind > > as to verify this change with your Areca hardware if you still have it? > > It's been up in the loft for years, but I'll get it out this weekend and > give it a spin :) this'll let me make sure the disks still spin as well, > which matters for an in-case-of-lightning-strike disaster-recovery > backup box. > > (I just hope this kernel boots on it at all. It's about three years > since I retired it... let's see!) FWIW if all else fails you can try this patch with the original kernel you used with the box. This piece of code hasn't changed, so until I came up with the complete five-part solution proposed here I merely had the original commit reverted as it is so as to allow forward progress. In any case, as per the cover letter, I have upgraded from 2.6.18, much older, and this was the sole show-stopper for the machine, running SMP even, so chances are 5.11+ will work with your system as well. The other plain 486/EISA/ATA box, similarly upgraded (now that I got its faulty odd industrial PSU finally replaced) works just fine with vanilla 5.11. OTOH versions ~3.15 through to ~4.5 I have tried while bisecting this issue mostly failed to even start booting due to what looks like a heisenbug to me (e.g. switching from XZ to gzip for compression would make some, but not all versions/configurations boot occasionally), so YMMV. Overall we're not that bad with keeping stuff working, it's more new use that causes troubles sometimes. > > It looks to me like you were thinking in the right direction with: > > <https://lore.kernel.org/linux-scsi/87vc3nuipg.fsf@spindle.srvr.nix/>. > > It's the sort of mistake I could see myself making: an easy mistake to > make when so many things in C require buffer size - 1 or you get a > disastrous security hole... And here it's masking, except that with (256 - 1) rather than (512 - 1) as you suggested. Thank you for your input! Maciej
Index: linux-macro-ide/drivers/scsi/sd.c =================================================================== --- linux-macro-ide.orig/drivers/scsi/sd.c +++ linux-macro-ide/drivers/scsi/sd.c @@ -3076,16 +3076,13 @@ static void sd_read_write_same(struct sc } if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE, INQUIRY) < 0) { - /* too large values might cause issues with arcmsr */ - int vpd_buf_len = 64; - sdev->no_report_opcodes = 1; /* Disable WRITE SAME if REPORT SUPPORTED OPERATION * CODES is unsupported and the device has an ATA * Information VPD page (SAT). */ - if (!scsi_get_vpd_page(sdev, 0x89, buffer, vpd_buf_len)) + if (!scsi_get_vpd_page(sdev, 0x89, buffer, SCSI_VPD_PG_LEN)) sdev->no_write_same = 1; }
Set the allocation length to 255 for the ATA Information VPD page requested in the WRITE SAME handler, so as not to limit information examined by `scsi_get_vpd_page' in the supported vital product data pages unnecessarily. Originally it was thought that Areca hardware may have issues with a valid allocation length supplied for a VPD inquiry, however older SCSI standard revisions[1] consider 255 the maximum length allowed and what has later become the high order byte is considered reserved and must be zero with the INQUIRY command. Therefore it was unnecessary to reduce the amount of data requested from 512 as far down as to 64, arbitrarily chosen, and 255 would as well do. With commit b3ae8780b429 ("[SCSI] Add EVPD page 0x83 and 0x80 to sysfs") we have since got the SCSI_VPD_PG_LEN macro, so use that instead. References: [1] "Information technology - Small Computer System Interface - 2", WORKING DRAFT, X3T9.2, Project 375D, Revision 10L, 7-SEP-93, Section 8.2.5 "INQUIRY command", pp.104-108 Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> Fixes: af73623f5f10 ("[SCSI] sd: Reduce buffer size for vpd request") --- Nix, I can see you're still around. Would you therefore please be so kind as to verify this change with your Areca hardware if you still have it? It looks to me like you were thinking in the right direction with: <https://lore.kernel.org/linux-scsi/87vc3nuipg.fsf@spindle.srvr.nix/>. Sadly nobody seemed to have paid attention to your observation and neither were different buffer sizes considered (or at least it wasn't mentioned in the discussion). Maciej --- drivers/scsi/sd.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) linux-scsi-write-same-vpd-buffer.diff