Re: [PATCH] loop: zero fill bio instead of return -EIO for partial read

[Dave Young <dyoung@redhat.com>]

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)



-- 
Thanks
Dave