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]

Date:   Sat, 16 Nov 2019 17:30:32 +1300
From:   Michael Schmitz <schmitzmic@...il.com>
To:     Finn Thain <fthain@...egraphics.com.au>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>
Cc:     Kars de Jong <jongk@...ux-m68k.org>, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] esp_scsi: Clear Transfer Count registers before PIO
 transfers

Hi Finn,

Am 16.11.2019 um 16:36 schrieb Finn Thain:
> 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.

I believe you also need to send a DMA NOP command to the ESP:

"Values
written to these two registers will be stored inter-
nally and loaded into the transfer counter by any
DMA command. These values remain unchanged
while the transfer counter decrements. Thus,
successive blocks of equal size may be transferred
without reprogramming the count. They may be
reprogrammed any time after the previous DMA
operation has started, whether it has finished or
not."

"Any DMA command will load the transfer count into
the counter. A DMA NOP (80h) will load the
counter while the non-DMA NOP (OOh) will not."

(53CF64_96-2 data book)

Without the DMA NOP, the transfer counts will remain as they were left 
by the prematurely terminated DMA command.

Cheers,

	Michael

>
> Cc: Michael Schmitz <schmitzmic@...il.com>
> Cc: Kars de Jong <jongk@...ux-m68k.org>
> Tested-by: Stan Johnson <userm57@...oo.com>
> Signed-off-by: Finn Thain <fthain@...egraphics.com.au>
> ---
>  drivers/scsi/esp_scsi.c | 3 +++
>  drivers/scsi/mac_esp.c  | 2 --
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/esp_scsi.c b/drivers/scsi/esp_scsi.c
> index bb88995a12c7..afbef83f5dd9 100644
> --- a/drivers/scsi/esp_scsi.c
> +++ b/drivers/scsi/esp_scsi.c
> @@ -2835,6 +2835,9 @@ void esp_send_pio_cmd(struct esp *esp, u32 addr, u32 esp_count,
>  	cmd &= ~ESP_CMD_DMA;
>  	esp->send_cmd_error = 0;
>
> +	esp_write8(0, ESP_TCLOW);
> +	esp_write8(0, ESP_TCMED);
> +
>  	if (write) {
>  		u8 *dst = (u8 *)addr;
>  		u8 mask = ~(phase == ESP_MIP ? ESP_INTR_FDONE : ESP_INTR_BSERV);
> diff --git a/drivers/scsi/mac_esp.c b/drivers/scsi/mac_esp.c
> index 1c78bc10c790..797579247e47 100644
> --- a/drivers/scsi/mac_esp.c
> +++ b/drivers/scsi/mac_esp.c
> @@ -361,8 +361,6 @@ static int esp_mac_probe(struct platform_device *dev)
>  	esp->flags = ESP_FLAG_NO_DMA_MAP;
>  	if (mep->pdma_io == NULL) {
>  		printk(KERN_INFO PFX "using PIO for controller %d\n", dev->id);
> -		esp_write8(0, ESP_TCLOW);
> -		esp_write8(0, ESP_TCMED);
>  		esp->flags |= ESP_FLAG_DISABLE_SYNC;
>  		mac_esp_ops.send_dma_cmd = esp_send_pio_cmd;
>  	} else {
>