| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 13 Jan 2012 15:38:04 +0800
From: Dave Young <dyoung@...hat.com>
To: Jeff Moyer <jmoyer@...hat.com>
CC: axboe@...nel.dk, linux-kernel@...r.kernel.org
Subject: Re: [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@...hat.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@...hat.com>
>>
>>>
>>> Signed-off-by: Dave Young <dyoung@...hat.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists