lkml.org 
[lkml]   [2012]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] loop: zero fill bio instead of return -EIO for partial read
    On 01/08/2012 09:54 PM, Dave Young wrote:

    > On 01/07/2012 03:22 AM, Jeff Moyer wrote:
    >
    >> Dave Young <dyoung@redhat.com> writes:
    >>
    >>> commit 8268f5a7415d914fc855a86aa2284ac819dc6b2e trying to fix the loop device
    >>> partial read information leak problem. But it changed the semantics of read
    >>> behavior. When we read beyond the end of the device we should get 0 bytes,
    >>> which is normal behavior, we should not just return -EIO
    >>>
    >>> Instead of return -EIO, zero out the bio to avoid information leak in case of
    >>> partail read.
    >>
    >> I tested the patch with a program which patterns the loop device,
    >> truncates the backing file, and then performs preads from various
    >> offsets within the loop device, validates the return values and inspects
    >> the contents. With this patch, everything works as expected.
    >
    >
    > Many thanks for review and test
    >
    >>
    >> By the way, truncating the backing file for a loop device is insane.
    >> Why would you do that? Also, if you really want all of the data gone,
    >> you'll have to flush the contents of the buffer cache for the loop
    >> device first. It's quite the head scratcher the first time you truncate
    >> a file, wait a few seconds, and then witness the file size grow. ;-)
    >
    >
    > I think nobody will intend to do that, but random operations could cause
    > this happen..
    >
    >>
    >> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
    >>
    >>>
    >>> Signed-off-by: Dave Young <dyoung@redhat.com>
    >>> ---
    >>> drivers/block/loop.c | 24 +++++++++++++-----------
    >>> 1 file changed, 13 insertions(+), 11 deletions(-)
    >>>
    >>> --- linux-2.6.orig/drivers/block/loop.c 2012-01-06 11:19:48.000000000 +0800
    >>> +++ linux-2.6/drivers/block/loop.c 2012-01-06 11:20:18.842630580 +0800
    >>> @@ -357,14 +357,14 @@ lo_direct_splice_actor(struct pipe_inode
    >>> return __splice_from_pipe(pipe, sd, lo_splice_actor);
    >>> }
    >>>
    >>> -static int
    >>> +static ssize_t
    >>> do_lo_receive(struct loop_device *lo,
    >>> struct bio_vec *bvec, int bsize, loff_t pos)
    >>> {
    >>> struct lo_read_data cookie;
    >>> struct splice_desc sd;
    >>> struct file *file;
    >>> - long retval;
    >>> + ssize_t retval;
    >>>
    >>> cookie.lo = lo;
    >>> cookie.page = bvec->bv_page;
    >>> @@ -380,26 +380,28 @@ do_lo_receive(struct loop_device *lo,
    >>> file = lo->lo_backing_file;
    >>> retval = splice_direct_to_actor(file, &sd, lo_direct_splice_actor);
    >>>
    >>> - if (retval < 0)
    >>> - return retval;
    >>> - if (retval != bvec->bv_len)
    >>> - return -EIO;
    >>> - return 0;
    >>> + return retval;
    >>> }
    >>>
    >>> static int
    >>> lo_receive(struct loop_device *lo, struct bio *bio, int bsize, loff_t pos)
    >>> {
    >>> struct bio_vec *bvec;
    >>> - int i, ret = 0;
    >>> + ssize_t s;
    >>> + int i;
    >>>
    >>> bio_for_each_segment(bvec, bio, i) {
    >>> - ret = do_lo_receive(lo, bvec, bsize, pos);
    >>> - if (ret < 0)
    >>> + s = do_lo_receive(lo, bvec, bsize, pos);
    >>> + if (s < 0)
    >>> + return s;
    >>> +
    >>> + if (s != bvec->bv_len) {
    >>> + zero_fill_bio(bio);
    >>> break;
    >>> + }
    >>> pos += bvec->bv_len;
    >>> }
    >>> - return ret;
    >>> + return 0;
    >>> }
    >>>
    >>> static int do_bio_filebacked(struct loop_device *lo, struct bio *bio)
    >
    >
    >


    Jens, what do you think about this patch? Could you take this?

    --
    Thanks
    Dave


    \
     
     \ /
      Last update: 2012-01-13 08:37    [W:0.028 / U:30.216 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site