lkml.org 
[lkml]   [2019]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] [PATCH v2] fsi: fix bogus error returns from cfam_read and cfam_write
Date
Based on a static analysis report and original patch from Colin Ian King
<colin.king@canonical.com>.

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@canonical.com>
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---
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
\
 
 \ /
  Last update: 2019-11-29 04:25    [W:0.033 / U:0.984 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site