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

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] [day] [month] [year] [list]

Date:   Mon, 10 Jan 2022 10:00:59 -0600
From:   Eddie James <eajames@...ux.ibm.com>
To:     linux-fsi@...ts.ozlabs.org
Cc:     linux-kernel@...r.kernel.org, joel@....id.au, jk@...abs.org,
        alistair@...ple.id.au
Subject: Re: [PATCH] fsi: occ: Fetch OCC response header again to avoid
 checksum failure


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@...ux.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;
>   	}