All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@orcam.me.uk>
To: Nix <nix@esperi.org.uk>
Cc: Khalid Aziz <khalid@gonehiking.org>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Christoph Hellwig <hch@lst.de>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/5] scsi: Set allocation length to 255 for ATA Information VPD page
Date: Fri, 16 Apr 2021 17:18:20 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.21.2104161628540.44318@angie.orcam.me.uk> (raw)
In-Reply-To: <878s5joh2d.fsf@esperi.org.uk>

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

  reply	other threads:[~2021-04-16 15:19 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-14 22:38 [PATCH 0/5] Bring the BusLogic host bus adapter driver up to Y2021 Maciej W. Rozycki
2021-04-14 22:39 ` [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use Maciej W. Rozycki
2021-04-16  2:08   ` Joe Perches
2021-04-16 10:48     ` Maciej W. Rozycki
2021-04-16 14:02       ` Joe Perches
2021-04-16 14:28         ` Maciej W. Rozycki
2021-04-16 15:12           ` Joe Perches
2021-04-17 11:39       ` David Laight
2021-04-17 14:01         ` Maciej W. Rozycki
2021-04-16 20:41     ` Khalid Aziz
2021-04-17  0:09       ` Joe Perches
2021-04-16 19:34   ` Khalid Aziz
2021-04-14 22:39 ` [PATCH 2/5] scsi: BusLogic: Avoid unbounded `vsprintf' use Maciej W. Rozycki
2021-04-16 19:54   ` Khalid Aziz
2021-04-14 22:39 ` [PATCH 3/5] scsi: Provide for avoiding trailing allocation length with VPD inquiries Maciej W. Rozycki
2021-04-14 22:39 ` [PATCH 4/5] scsi: Avoid using reserved length byte " Maciej W. Rozycki
2021-04-14 22:39 ` [PATCH 5/5] scsi: Set allocation length to 255 for ATA Information VPD page Maciej W. Rozycki
2021-04-15 12:42   ` Nix
2021-04-16 15:18     ` Maciej W. Rozycki [this message]
2021-04-16 19:36 ` [PATCH 0/5] Bring the BusLogic host bus adapter driver up to Y2021 Khalid Aziz
2021-04-16 21:25   ` Maciej W. Rozycki
2021-04-18 20:21     ` Ondrej Zary
2021-04-19 15:06       ` Khalid Aziz
2021-04-19 16:01         ` Maciej W. Rozycki
2021-04-20  2:16           ` Khalid Aziz
2021-04-20 18:02             ` Maciej W. Rozycki
2021-04-22  2:36               ` Khalid Aziz
2021-04-22 16:27                 ` Maciej W. Rozycki
2021-04-22 18:07                   ` Khalid Aziz
2021-04-22 23:19                     ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.2104161628540.44318@angie.orcam.me.uk \
    --to=macro@orcam.me.uk \
    --cc=hch@lst.de \
    --cc=jejb@linux.ibm.com \
    --cc=khalid@gonehiking.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=nix@esperi.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.