block: cciss: fix information leak to userland

PATCH In review Vasiliy Kulikov 3 participants 5 messages Oct 23, 2010 – Oct 27, 2010

Subsystems (2)

Block layer Kernel janitors

lore

Share & tools

lore

Vasiliy Kulikov

author

2010-10-23 08:25 UTC #0 lore raw

PATCH 1/2 block: cciss: fix information leak to userland

Structure IOCTL_Command_struct is copied to userland with some padding fields at the end of the struct unitialized. It leads to leaking of stack memory.

Signed-off-by Vasiliy Kulikov <[email protected]>

---
 drivers/block/cciss.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/block/cciss.c +1 -0

index 762a81a..cd08324 100644
@@ -1142,6 +1142,7 @@ static int cciss_ioctl32_passthru(struct block_device *bdev, fmode_t mode,
 	int err;
 	u32 cp;
 
+	memset(&arg64, 0, sizeof(arg64));
 	err = 0;
 	err | 	    copy_from_user(&arg64.LUN_info, &arg32->LUN_info,

Signature · 1 line

1.7.0.4

I would think hpsa needs a similar fix, and also cciss_ioctl32_big_passthru(). I suppose it's possible the big passthru alignment just "works out", (I haven't checked), but even if that is the case, probably best not to rely on such an accident,

-- steve

Miller, Mike (OS Dev)

Acked-by on v1 1/2

2010-10-25 14:40 UTC #2 lore raw

In reply to Vasiliy Kulikov

Quoted 24 lines

-----Original Message-----
From: Vasiliy Kulikov [mailto:[email protected]]
Sent: Saturday, October 23, 2010 3:25 AM
To: [email protected]
Cc: Miller, Mike (OS Dev); Jens Axboe; Stephen M. Cameron; Andrew Morton; ISS StorageDev; [email protected]
Subject: [PATCH 1/2] block: cciss: fix information leak to userland
Structure IOCTL_Command_struct is copied to userland with some padding fields at the end of the struct unitialized. It leads to leaking of stack memory.
Signed-off-by Vasiliy Kulikov <[email protected]>
---
 drivers/block/cciss.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/block/cciss.c +0 -0
index 762a81a..cd08324 100644

drivers/block/cciss.c static int

Quoted 8 lines

drivers/block/cciss.c +1 -1

@@ -1142,6 +1142,7 @@ static int 
 cciss_ioctl32_passthru(struct block_device *bdev, fmode_t mode,
 	int err;
 	u32 cp;
 
+	memset(&arg64, 0, sizeof(arg64));
 	err = 0;
 	err |>  	    copy_from_user(&arg64.LUN_info, &arg32->LUN_info,
--

Acked-by Mike Miller <[email protected]>

Quoted 3 lines

1.7.0.4

Miller, Mike (OS Dev)

2010-10-25 16:36 UTC #3 lore raw

Quoted 38 lines

-----Original Message-----
From: Miller, Mike (OS Dev)
Sent: Monday, October 25, 2010 9:41 AM
To: Vasiliy Kulikov; [email protected]
Cc: Jens Axboe; Stephen M. Cameron; Andrew Morton; ISS StorageDev; [email protected]
Subject: RE: [PATCH 1/2] block: cciss: fix information leak to userland
Quoted 26 lines
-----Original Message-----
From: Vasiliy Kulikov [mailto:[email protected]]
Sent: Saturday, October 23, 2010 3:25 AM
To: [email protected]
Cc: Miller, Mike (OS Dev); Jens Axboe; Stephen M. Cameron; Andrew Morton; ISS StorageDev; [email protected]
Subject: [PATCH 1/2] block: cciss: fix information leak to userland
Structure IOCTL_Command_struct is copied to userland with some padding fields at the end of the struct unitialized. It leads to leaking of stack memory.
Signed-off-by Vasiliy Kulikov <[email protected]>
---
 drivers/block/cciss.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/block/cciss.c +0 -0
index 762a81a..cd08324 100644
@@ -1142,6 +1142,7 @@ static int
 cciss_ioctl32_passthru(struct block_device *bdev, fmode_t mode,
 	int err;
 	u32 cp;
 

Quoted 6 lines

drivers/block/cciss.c static int cciss_ioctl32_passthru(struct block_device *bdev, fmode_t mode,
Quoted 4 lines
drivers/block/cciss.c +1 -1
+	memset(&arg64, 0, sizeof(arg64));
 	err = 0;
 	err |> >  	    copy_from_user(&arg64.LUN_info, &arg32->LUN_info,
--
Acked-by Mike Miller <[email protected]>

BTW: this change needs to go into the cciss_ioctl32_big_passthru and the other passthru ioctls, also.

-- mikem

Quoted 4 lines

Quoted 4 lines
1.7.0.4

Vasiliy Kulikov

author

2010-10-27 15:43 UTC #4 lore raw

Hi Mike,

Quoted 38 lines · Mike (OS Dev)

Quoted 38 lines
-----Original Message-----
From: Miller, Mike (OS Dev)
Sent: Monday, October 25, 2010 9:41 AM
To: Vasiliy Kulikov; [email protected]
Cc: Jens Axboe; Stephen M. Cameron; Andrew Morton; ISS StorageDev; [email protected]
Subject: RE: [PATCH 1/2] block: cciss: fix information leak to userland
Quoted 26 lines
-----Original Message-----
From: Vasiliy Kulikov [mailto:[email protected]]
Sent: Saturday, October 23, 2010 3:25 AM
To: [email protected]
Cc: Miller, Mike (OS Dev); Jens Axboe; Stephen M. Cameron; Andrew Morton; ISS StorageDev; [email protected]
Subject: [PATCH 1/2] block: cciss: fix information leak to userland
Structure IOCTL_Command_struct is copied to userland with some padding fields at the end of the struct unitialized. It leads to leaking of stack memory.
Signed-off-by Vasiliy Kulikov <[email protected]>
---
 drivers/block/cciss.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/block/cciss.c +0 -0
index 762a81a..cd08324 100644
@@ -1142,6 +1142,7 @@ static int
 cciss_ioctl32_passthru(struct block_device *bdev, fmode_t mode,
 	int err;
 	u32 cp;
 

Quoted 8 lines

Quoted 6 lines
drivers/block/cciss.c static int cciss_ioctl32_passthru(struct block_device *bdev, fmode_t mode,
Quoted 4 lines
drivers/block/cciss.c +1 -1
+	memset(&arg64, 0, sizeof(arg64));
 	err = 0;
 	err |> > >  	    copy_from_user(&arg64.LUN_info, &arg32->LUN_info,
--
Acked-by Mike Miller <[email protected]>
BTW: this change needs to go into the cciss_ioctl32_big_passthru

I've sent the patch, thanks.

Quoted 1 line

and the other passthru ioctls, also.

I don't see problems with them, they do copy_from_user() before filling the structure.

Quoted 7 lines

-- mikem
Quoted 4 lines
Quoted 4 lines
1.7.0.4

Thanks,

Signature · 1 line

Vasiliy