Based on a static analysis report and original patch from Colin Ian King <colin.king@xxxxxxxxxxxxx>. Currently, we may drop error values from cfam_read and cfam_write. This change returns the actual error on failure, but a partial read/write will take precedence. Addresses-Coverity: ("Unused value") Fixes: d1dcd6782576 ("fsi: Add cfam char devices") Reported-by: Colin Ian King <colin.king@xxxxxxxxxxxxx> Signed-off-by: Jeremy Kerr <jk@xxxxxxxxxx> --- Colin: thanks for the report and patch. I think this is a more complete fix, as we want to preseve any partial read/write status if a failure happens mid-way through an operation. Let me know if you (or the coverity analysis) have any feedback. --- drivers/fsi/fsi-core.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index 71c6f9fef648..3158a78c2e94 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -699,6 +699,8 @@ static ssize_t cfam_read(struct file *filep, char __user *buf, size_t count, if (off > 0xffffffff || count > 0xffffffff || off + count > 0xffffffff) return -EINVAL; + rc = 0; + for (total_len = 0; total_len < count; total_len += read_len) { __be32 data; @@ -707,18 +709,22 @@ static ssize_t cfam_read(struct file *filep, char __user *buf, size_t count, rc = fsi_slave_read(slave, off, &data, read_len); if (rc) - goto fail; + break; rc = copy_to_user(buf + total_len, &data, read_len); if (rc) { rc = -EFAULT; - goto fail; + break; } off += read_len; } - rc = count; - fail: + + /* if we've read any data, we want that to be returned in + * preference to an error state */ + if (total_len) + rc = total_len; + *offset = off; - return count; + return rc; } static ssize_t cfam_write(struct file *filep, const char __user *buf, @@ -736,6 +742,8 @@ static ssize_t cfam_write(struct file *filep, const char __user *buf, if (off > 0xffffffff || count > 0xffffffff || off + count > 0xffffffff) return -EINVAL; + rc = 0; + for (total_len = 0; total_len < count; total_len += write_len) { __be32 data; @@ -745,17 +753,21 @@ static ssize_t cfam_write(struct file *filep, const char __user *buf, rc = copy_from_user(&data, buf + total_len, write_len); if (rc) { rc = -EFAULT; - goto fail; + break; } rc = fsi_slave_write(slave, off, &data, write_len); if (rc) - goto fail; + break; off += write_len; } - rc = count; - fail: + + /* if we've written any data, we want to indicate that partial write + * instead of any mid-stream error */ + if (total_len) + rc = total_len; + *offset = off; - return count; + return rc; } static loff_t cfam_llseek(struct file *file, loff_t offset, int whence) -- 2.20.1