Hi Dmitry, On Fri, 2017-03-31 at 19:53 +0400, Dmitry Monakhov wrote: > 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. > > testcase: https://github.com/dmonakhov/xfstests/commit/e11a1b7b907ca67b1be51a1594025600767366d5 > Signed-off-by: Dmitry Monakhov <dmonakhov@xxxxxxxxxx> > --- > drivers/target/target_core_file.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > Nice catch on this one. Just a small comment below. > diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c > index 87aa376..d69908d 100644 > --- a/drivers/target/target_core_file.c > +++ b/drivers/target/target_core_file.c <SNIP> > @@ -295,17 +294,27 @@ 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. > + */ > + BUG_ON(ret > data_length); > + ret += iov_iter_zero(data_length - ret, &iter); > } A BUG_ON for this is overkill. No need to kill the whole node. ;) Applying + squashing the follow atop your original patch to just return -EINVAL and fail se_cmd instead. Thank you, diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index d69908d..dd8f320 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -308,8 +308,10 @@ static int fd_do_rw(struct se_cmd *cmd, struct file *fd, * We must explicitly zero sg-pages to prevent * expose uninizialized pages to userspace. */ - BUG_ON(ret > data_length); - ret += iov_iter_zero(data_length - ret, &iter); + if (ret < data_length) + ret += iov_iter_zero(data_length - ret, &iter); + else + ret = -EINVAL; } } } -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
