linux-kernel - Re: [PATCH] esp_scsi: Clear Transfer Count registers before PIO transfers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LNX.2.21.1.1911180947020.8@nippy.intranet>
Date:   Mon, 18 Nov 2019 10:13:08 +1100 (AEDT)
From:   Finn Thain <fthain@...egraphics.com.au>
To:     Kars de Jong <jongk@...ux-m68k.org>
cc:     "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Michael Schmitz <schmitzmic@...il.com>,
        linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] esp_scsi: Clear Transfer Count registers before PIO
 transfers

On Sun, 17 Nov 2019, Kars de Jong wrote:

> Hi Finn,
> 
> Op za 16 nov. 2019 om 04:36 schreef Finn Thain 
> <fthain@...egraphics.com.au>:
> >
> > The zorro_esp driver uses both PIO and DMA transfers. If a failed DMA 
> > transfer happened to be followed by a PIO transfer, the TCLOW and 
> > TCMED registers would not get cleared. It is theoretically possible 
> > that the stale value from the transfer counter or the TCLOW/TCMED 
> > registers could then be used by the controller and the driver. Avoid 
> > that by clearing these registers before each PIO transfer.
> 
> Are you sure this is really needed?
> 

No. I think it improves robustness and correctness.

I would be interested to know whether there is any measurable performance 
impact on zorro_esp.

> The only [time when] the driver reads these registers is after a data 
> transfer. These are done using DMA on all Zorro boards, so I don't think 
> there's a risk of stale values from a PIO transfer there.
> 

I'm not entirely sure that the chip is unaffected by stale counter values. 

(Stale transfer counter values are distinct from stale transfer count 
register values. Both are addressed by the patch.)

If there are DMA controllers out there that can't do very short transfers 
then this objection would seem to be invalid, because the "DMA length is 
zero!" issue could be tackled using PIO.

> The only place the controller reads these registers is when a DMA 
> command is issued. The only place where that is done is in the zorro_esp 
> send_dma_command() functions. 

Aren't you overlooking all of the ESP_CMD_DMA flags in the core driver?

Thanks for your review.

-- 

> These all set both registers explicitly before issuing the DMA command 
> to the controller, so I don't think there's a risk there either.
> 
> Kind regards,
> 
> Kars.
>