Re: [PATCH] fsi: occ: Fetch OCC response header again to avoid checksum failure

[Eddie James <eajames@linux.ibm.com>]

On 1/5/22 14:23, Eddie James wrote:
> In the event that the driver state is reset, and the previous OCC
> operation had a sequence number of 1, there is the possibility that
> the SRAM buffer is updated in between fetching the OCC response header
> and the rest of the data (since the sequence number match is really a
> false positive). This results in a checksum failure. To avoid this
> condition, simply fetch the whole response rather than skipping the
> header when fetching the rest of the response.


This can be abandoned in favor of the latest patch, "Improve response 
status checking"

Thanks,

Eddie


>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>   drivers/fsi/fsi-occ.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
> index 7eaab1be0aa4..660c3fc43be5 100644
> --- a/drivers/fsi/fsi-occ.c
> +++ b/drivers/fsi/fsi-occ.c
> @@ -546,10 +546,15 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
>   	dev_dbg(dev, "resp_status=%02x resp_data_len=%d\n",
>   		resp->return_status, resp_data_length);
>   
> -	/* Grab the rest */
> +	/*
> +	 * Grab the rest, including the occ response header again, just in case
> +	 * it changed in between our two getsram operations (this can happen
> +	 * despite the sequence number check if the driver state is reset). The
> +	 * data length shouldn't change at OCC runtime, and the response
> +	 * status, which may have changed, has to be checked by users anyway.
> +	 */
>   	if (resp_data_length > 1) {
> -		/* already got 3 bytes resp, also need 2 bytes checksum */
> -		rc = occ_getsram(occ, 8, &resp->data[3], resp_data_length - 1);
> +		rc = occ_getsram(occ, 0, resp, resp_data_length + 7);
>   		if (rc)
>   			goto done;
>   	}