| Message ID | 20220105202313.27510-1-eajames@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | fsi: occ: Fetch OCC response header again to avoid checksum failure | expand |
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; > }
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; }
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. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/fsi/fsi-occ.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)