Skip to contentSkip to conversation

firewire: info leak in ioctl_get_info()

PATCH In review Dan Carpenter 3 participants 4 messages Oct 19, 2013

Subsystems (2)
Kernel janitors Firewire
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 firewire: info leak in ioctl_get_info()

There is a 4 byte hole in the bus_reset struct at the end of the struct after ->generation.

Signed-off-by Dan Carpenter <[email protected]>
drivers/firewire/core-cdev.c +2 -0
index d7d5c8a..d34adb3 100644
@@ -353,6 +353,8 @@ static void fill_bus_reset_event(struct fw_cdev_event_bus_reset *event,
 {
 	struct fw_card *card = client->device->card;
 
+	memset(event, 0, sizeof(*event));
+
 	spin_lock_irq(&card->lock);
 
 	event->closure	     = client->bus_reset_closure;
#1 lore raw
Quoted 2 lines · Dan Carpenter

There is a 4 byte hole in the bus_reset struct at the end of the struct after ->generation.

queue_bus_reset_event() uses kzalloc(), and ioctl_get_info() does not copy the hole.

Quoted 7 lines
Signed-off-by Dan Carpenter <[email protected]>
drivers/firewire/core-cdev.c +0 -0
index d7d5c8a..d34adb3 100644

drivers/firewire/core-cdev.c static void fill_bus_reset_event(struct fw_cdev_event_bus_reset *event, · line 360

Quoted 8 lines
drivers/firewire/core-cdev.c +2 -0
@@ -353,6 +353,8 @@ static void fill_bus_reset_event(struct fw_cdev_event_bus_reset *event,
 {
 	struct fw_card *card = client->device->card;
 
+	memset(event, 0, sizeof(*event));
+
 	spin_lock_irq(&card->lock);
 
 	event->closure	     = client->bus_reset_closure;

Regards,
Clemens

#2 lore raw
In reply to Dan Carpenter
Quoted 6 lines · On Oct 19 Clemens Ladisch wrote:
Quoted 2 lines · Dan Carpenter

There is a 4 byte hole in the bus_reset struct at the end of the struct after ->generation.

queue_bus_reset_event() uses kzalloc(), and ioctl_get_info() does not copy the hole.

Yep. In other words, the ioctl_get_info() information leak has been fixed by commit 790198f74c9d "firewire: cdev: fix user memory corruption (i386 userland on amd64 kernel)". Its subject doesn't say so, but the changelog does.

Signature · 3 lines
Stefan Richter
-===-==-= =-=- =--=
http://arcgraph.de/sr/
author
#3 lore raw
In reply to Dan Carpenter
Quoted 7 lines · Clemens Ladisch
Quoted 2 lines · Dan Carpenter

There is a 4 byte hole in the bus_reset struct at the end of the struct after ->generation.

queue_bus_reset_event() uses kzalloc(), and ioctl_get_info() does not copy the hole.

Gar... Sorry you are right. I appologize for that.

regards,
dan carpenter