tcm_fileio: Prevent information leak for short reads

If we failed to read data from backing file (probably because some one
truncate file under us), we must zerofill cmd's data, otherwise it will
be returned as is. Most likely cmd's data are unitialized pages from
page cache. This result in information leak.

(Change BUG_ON into -EINVAL se_cmd failure - nab)

testcase: https://github.com/dmonakhov/xfstests/commit/e11a1b7b907ca67b1be51a1594025600767366d5
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>

diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 87aa376a1a1ae9f9119369725199d7bd5ba22a1e..dd8f32055266def613462c205e4e528f7933455c 100644 (file)
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -277,12 +277,11 @@ static int fd_do_rw(struct se_cmd *cmd, struct file *fd,
        else
                ret = vfs_iter_read(fd, &iter, &pos);
 
-       kfree(bvec);
-
        if (is_write) {
                if (ret < 0 || ret != data_length) {
                        pr_err("%s() write returned %d\n", __func__, ret);
-                       return (ret < 0 ? ret : -EINVAL);
+                       if (ret >= 0)
+                               ret = -EINVAL;
                }
        } else {
                /*
@@ -295,17 +294,29 @@ static int fd_do_rw(struct se_cmd *cmd, struct file *fd,
                                pr_err("%s() returned %d, expecting %u for "
                                                "S_ISBLK\n", __func__, ret,
                                                data_length);
-                               return (ret < 0 ? ret : -EINVAL);
+                               if (ret >= 0)
+                                       ret = -EINVAL;
                        }
                } else {
                        if (ret < 0) {
                                pr_err("%s() returned %d for non S_ISBLK\n",
                                                __func__, ret);
-                               return ret;
+                       } else if (ret != data_length) {
+                               /*
+                                * Short read case:
+                                * Probably some one truncate file under us.
+                                * We must explicitly zero sg-pages to prevent
+                                * expose uninizialized pages to userspace.
+                                */
+                               if (ret < data_length)
+                                       ret += iov_iter_zero(data_length - ret, &iter);
+                               else
+                                       ret = -EINVAL;
                        }
                }
        }
-       return 1;
+       kfree(bvec);
+       return ret;
 }
 
 static sense_reason_t