IronPort-SDR: DLVYw+ZuQaAv0hcUmk8WLMh1FRtgjXc6kMXdGo+PVWXfd8XT1xwFTx7EV/9k+ttHSH00+c/OTM
 ctfjFpsZuyt/9LgtQiQWCGm4sWeAelD30oV4Lakm81mM3Dga+QSJH7yyOh+EI4J0cACnJZ6DVo
 ZfvMDyDNvXki5C12i/3Ncu4Mgz+iCkY0Wt68mBb7WnaLfW8tO0niZTsDmlxfc4O9kXd9oEbYJx
 V9/ZUMc86qQlPQeBohzuv5idoS0oHrjg4SdVnuuM5YuHELn9cDA+lXh40Bdqqcp37LrUhu4pB7
 OGYnCitc+D70pSZn9d1oVtEm
IronPort-SDR: SVLrRnUfx6EexsjjeBw2VHGwkytmStBBWqz65FA27+a5yA42Hc1G4fAywov6U4iKHRhQsGa0sr
 RZQJ9yWV23Cet5cCPN84PiE4R0fHkreDm2+3uhh2rAW1CzKEVZNcj4p9W0PNqbi259pxR+Xdj6
 9YjmyXMlHCw9/kuI3BZXLjGPYkQSWdEsMm7VjUoCfTS1mT3M38ENrfJGGbp+jb0L0iJgqLs2e2
 zHT10fpWxV0uGShGNLs5bKr9Z8LnM9y2uGubTXDWVQelUkn8XmX1aPSkHS+madiEDXVke7PyrT
 VS0=
WDCIronportException: Internal
Message-ID: <87d2345c-f878-1884-e344-25ac2b6862cd@opensource.wdc.com>
Date:   Wed, 15 Dec 2021 08:42:50 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.4.0
Subject: Re: [PATCH v2 RESEND] libata: if T_LENGTH is zero, dma direction
 should be DMA_NONE
Content-Language: en-US
To:     George Kennedy <george.kennedy@oracle.com>
Cc:     linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
References: <1639493110-15900-1-git-send-email-george.kennedy@oracle.com>
From:   Damien Le Moal <damien.lemoal@opensource.wdc.com>
Organization: Western Digital
In-Reply-To: <1639493110-15900-1-git-send-email-george.kennedy@oracle.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2021/12/14 23:45, George Kennedy wrote:
> Avoid data corruption by rejecting pass-through commands where
> T_LENGTH is zero (No data is transferred) and the dma direction
> is not DMA_NONE.
> 
> Cc: <stable@vger.kernel.org>

George,

FYI, you should not send patches that need backporting to
stable@vger.kernel.org. The above Cc: tag will do that automatically.

> Reported-by: syzkaller<syzkaller@googlegroups.com>
> Signed-off-by: George Kennedy<george.kennedy@oracle.com>
> ---
> Used the Maintainers suggested fix.

The usual way of writing this is something like:

Changes from v1:
* Blah

This way, if you need a v3, v4, etc, you can keep the previous changes
description and add new ones on top.

> 
>  drivers/ata/libata-scsi.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 1b84d55..313e947 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2859,8 +2859,19 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
>  		goto invalid_fld;
>  	}
>  
> -	if (ata_is_ncq(tf->protocol) && (cdb[2 + cdb_offset] & 0x3) == 0)
> -		tf->protocol = ATA_PROT_NCQ_NODATA;
> +	if ((cdb[2 + cdb_offset] & 0x3) == 0) {
> +		/*
> +		 * When T_LENGTH is zero (No data is transferred), dir should
> +		 * be DMA_NONE.
> +		 */
> +		if (scmd->sc_data_direction != DMA_NONE) {
> +			fp = 2 + cdb_offset;
> +			goto invalid_fld;
> +		}
> +
> +		if (ata_is_ncq(tf->protocol))
> +			tf->protocol = ATA_PROT_NCQ_NODATA;
> +	}
>  
>  	/* enable LBA */
>  	tf->flags |= ATA_TFLAG_LBA;

This look OK to me. Will apply.


-- 
Damien Le Moal
Western Digital Research