mbox series

[v2,0/5] Bring the BusLogic host bus adapter driver up to Y2021

Message ID alpine.DEB.2.21.2104201934280.44318@angie.orcam.me.uk
Headers show
Series Bring the BusLogic host bus adapter driver up to Y2021 | expand

Message

Maciej W. Rozycki April 20, 2021, 6:01 p.m. UTC
Hi,

 This is v2 of the series with 2/5 updated to use `vscnprintf' rather than 
`vsnprintf'.  No other changes.

 So we are here owing to Christoph's recent ISA bounce buffering sweep: 
<https://lore.kernel.org/linux-scsi/20210331073001.46776-1-hch@lst.de/T/#m981284e74e93216626a0728ce1601ca18fca92e8> 
which has prompted me to verify the current version of Linux with my old 
server, which has been long equipped with venerable Linux 2.6.18 and which 
I now have available for general experimenting, and the BusLogic BT-958 
PCI SCSI host bus adapter the server has used for 20-something years now.  
This revealed numerous issues with the BusLogic driver.

 Firstly (1/5) it has suffered from some bitrot and messages produced have 
become messy from the lack of update for proper `pr_cont' support.

 Secondly (2/5) there has been a potential buffer overrun/stack corruption 
security issue from using an unbounded `vsprintf' call.

 Thirdly (3/5) it has become obvious the BusLogic driver would have been 
non-functional, should I have upgraded the kernel, at least with this 
configuration for some 8 years now, and the underlying cause has been a 
long-known issue with the MultiMaster firmware I have dealt with already, 
back in 2003.  To put it short the firmware cannot cope with commands that 
request an allocation length exceeding the length of actual data returned.

 I have originally observed it with a LOG SENSE command in the course of 
investigating why smartmontools bring the system to a death, and worked it 
around: <https://sourceforge.net/p/smartmontools/mailman/message/4993087/> 
by issuing the command twice, first just to obtain the allocation length 
required.  As it turns out we need a similar workaround in the kernel now.

 But in the course of investigating this issue I have discovered there is 
a second bottom to it and hence I have prepared follow-up changes (4-5/5) 
to address problems with our handling of Vital Product Data INQUIRY pages.

 See individual change descriptions for further details.

 Questions, comments, concerns?  Otherwise please apply.

  Maciej

Comments

David Laight April 21, 2021, 8:13 a.m. UTC | #1
From: Maciej W. Rozycki
> Sent: 20 April 2021 19:02
> 
> Update BusLogic driver's messaging system to use `pr_cont' for
> continuation lines, bringing messy output:

If reasonably possible it is best to avoid use of pr_cont().

If there are concurrent writes from multiple cpu I believe
the writes still get separated.
(Something has to give...)

For instance I've seen concurrent 'oops' generate the list of
loaded modules one module per line with the two lists interleaved!

Quite often one of the %p[A-Z] modifiers can help.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Maciej W. Rozycki April 21, 2021, 12:28 p.m. UTC | #2
On Wed, 21 Apr 2021, David Laight wrote:

> > Update BusLogic driver's messaging system to use `pr_cont' for
> > continuation lines, bringing messy output:
> 
> If reasonably possible it is best to avoid use of pr_cont().

 I know, however for that the whole driver's messaging system would have 
to be redesigned.  Joe (cc-ed) has offered to do it with the original 
iteration, however I believe consensus has been it will best be done as a 
separate follow-up change, while this small fix can be easily backported.

> If there are concurrent writes from multiple cpu I believe
> the writes still get separated.
> (Something has to give...)

 NB this driver may not see a lot of SMP use as I reckon it's had some 
portability issues, and in any case the hardware requires port I/O which 
precludes its use with some of the most recent PCIe systems which do not 
support PCI I/O cycles anymore.  And older systems were often UP.  Not 
that the driver should not be kept in a clean style of course.

  Maciej
Maciej W. Rozycki June 10, 2021, 11:25 p.m. UTC | #3
On Tue, 20 Apr 2021, Maciej W. Rozycki wrote:

>  Questions, comments, concerns?  Otherwise please apply.

 Ping for: 

<https://patchwork.kernel.org/project/linux-scsi/list/?series=470455&archive=both>.

 Where are we with this patch series?  I can see it's been archived in 
patchwork in the new state.  With the unexpected serial device fixes which 
preempted me and which I've just posted, moving them off the table I now 
have some spare cycles to get back here, but I'm not sure what to do.

 Nix was kind enough to verify and tell me off-list that 5/5 still works 
correctly with his system that required the earlier change referred there 
and the need for which was not completely understood back then -- Nix, are 
you OK with adding a `Tested-by' tag for your verification?

 Otherwise is there anything I need to do to move forward with these 
changes?  Shall I just repost the series as it was given that it still 
applies to Linus master verbatim?

  Maciej