From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753573Ab2AHNyY (ORCPT ); Sun, 8 Jan 2012 08:54:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53406 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752257Ab2AHNyX (ORCPT ); Sun, 8 Jan 2012 08:54:23 -0500 Message-ID: <4F09A006.8080104@redhat.com> Date: Sun, 08 Jan 2012 21:54:14 +0800 From: Dave Young User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.15) Gecko/20110323 Thunderbird/3.1.9 MIME-Version: 1.0 To: Jeff Moyer CC: axboe@kernel.dk, linux-kernel@vger.kernel.org Subject: Re: [PATCH] loop: zero fill bio instead of return -EIO for partial read References: <20120106033555.GA27916@darkstar.nay.redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/07/2012 03:22 AM, Jeff Moyer wrote: > Dave Young 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 > >> >> Signed-off-by: Dave Young >> --- >> 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) -- Thanks Dave