2010-11-04 14:48:45

by Kulikov Vasiliy

[permalink] [raw]
Subject: [PATCH] block: ioctl: fix information leak to userland

Structure hd_geometry is copied to userland with 4 padding bytes
between cylinders and start fields uninitialized on 64-bit platforms.
It leads to leaking of contents of kernel stack memory.

Currently there is no memset() in real implementations of getgeo()
in drivers/block/, so it makes sense to have memset() in blkdev_ioctl().

Signed-off-by: Vasiliy Kulikov <[email protected]>
---
block/ioctl.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index d724ceb..60bae81 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -242,6 +242,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
* We need to set the startsect first, the driver may
* want to override it.
*/
+ memset(&geo, 0, sizeof(geo));
geo.start = get_start_sect(bdev);
ret = disk->fops->getgeo(bdev, &geo);
if (ret)
--
1.7.0.4


2010-11-08 13:42:56

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] block: ioctl: fix information leak to userland

On 2010-11-04 15:48, Vasiliy Kulikov wrote:
> Structure hd_geometry is copied to userland with 4 padding bytes
> between cylinders and start fields uninitialized on 64-bit platforms.
> It leads to leaking of contents of kernel stack memory.
>
> Currently there is no memset() in real implementations of getgeo()
> in drivers/block/, so it makes sense to have memset() in blkdev_ioctl().

Thanks, applied.

--
Jens Axboe