bt8xx: info leak in ca_get_slot_info()

PATCH In review Dan Carpenter 2 participants 3 messages Jul 25, 2013

Subsystems (2)

Media (V4L/DVB) Kernel janitors

lore

Share & tools

lore

Dan Carpenter

author

2013-07-25 16:46 UTC #0 lore raw

PATCH 1/1 bt8xx: info leak in ca_get_slot_info()

p_ca_slot_info was allocated with kmalloc() so we need to clear it before passing it to the user.

Signed-off-by Dan Carpenter <[email protected]>

drivers/media/pci/bt8xx/dst_ca.c +4 -1

index 0e788fc..6b9dc3f 100644
@@ -302,8 +302,11 @@ static int ca_get_slot_info(struct dst_state *state, struct ca_slot_info *p_ca_s
 		p_ca_slot_info->flags = CA_CI_MODULE_READY;
 		p_ca_slot_info->num = 1;
 		p_ca_slot_info->type = CA_CI;
-	} else
+	} else {
 		p_ca_slot_info->flags = 0;
+		p_ca_slot_info->num = 0;
+		p_ca_slot_info->type = 0;
+	}
 
 	if (copy_to_user(arg, p_ca_slot_info, sizeof (struct ca_slot_info)))
 		return -EFAULT;

walter harms

2013-07-25 17:29 UTC #1 lore raw

Am 25.07.2013 18:46, schrieb Dan Carpenter:

Quoted 12 lines

p_ca_slot_info was allocated with kmalloc() so we need to clear it before passing it to the user.
Signed-off-by Dan Carpenter <[email protected]>
drivers/media/pci/bt8xx/dst_ca.c +0 -0
index 0e788fc..6b9dc3f 100644
@@ -302,8 +302,11 @@ static int ca_get_slot_info(struct dst_state *state, struct ca_slot_info *p_ca_s
 		p_ca_slot_info->flags = CA_CI_MODULE_READY;
 		p_ca_slot_info->num = 1;

drivers/media/pci/bt8xx/dst_ca.c static int ca_get_slot_info(struct dst_state *state, struct ca_slot_info *p_ca_s · line 312

Quoted 10 lines

drivers/media/pci/bt8xx/dst_ca.c +4 -1

 		p_ca_slot_info->type = CA_CI;
-	} else
+	} else {
 		p_ca_slot_info->flags = 0;
+		p_ca_slot_info->num = 0;
+		p_ca_slot_info->type = 0;
+	}
 
 	if (copy_to_user(arg, p_ca_slot_info, sizeof (struct ca_slot_info)))
 		return -EFAULT;

note: i have no clue how p_ca_slot_info looks like,
but to avoid information leaks via compiler padding etc. i could be more wise to do a memset(p_ca_slot_info,0,sizeof (struct ca_slot_info)) and then set the

	p_ca_slot_info->flags = CA_CI_MODULE_READY;
	p_ca_slot_info->num = 1;
	p_ca_slot_info->type = CA_CI;

just my 2 cents,
re,
wh

Dan Carpenter

author

2013-07-25 17:38 UTC #2 lore raw

Quoted 23 lines · walter harms

Am 25.07.2013 18:46, schrieb Dan Carpenter:
Quoted 20 lines
p_ca_slot_info was allocated with kmalloc() so we need to clear it before passing it to the user.
Signed-off-by Dan Carpenter <[email protected]>
drivers/media/pci/bt8xx/dst_ca.c +4 -1
index 0e788fc..6b9dc3f 100644
@@ -302,8 +302,11 @@ static int ca_get_slot_info(struct dst_state *state, struct ca_slot_info *p_ca_s
 		p_ca_slot_info->flags = CA_CI_MODULE_READY;
 		p_ca_slot_info->num = 1;
 		p_ca_slot_info->type = CA_CI;
-	} else
+	} else {
 		p_ca_slot_info->flags = 0;
+		p_ca_slot_info->num = 0;
+		p_ca_slot_info->type = 0;
+	}
 

Quoted 7 lines

drivers/media/pci/bt8xx/dst_ca.c static int ca_get_slot_info(struct dst_state *state, struct ca_slot_info *p_ca_s · line 312
Quoted 2 lines
drivers/media/pci/bt8xx/dst_ca.c +0 -0
 	if (copy_to_user(arg, p_ca_slot_info, sizeof (struct ca_slot_info)))
 		return -EFAULT;
note: i have no clue how p_ca_slot_info looks like,
but to avoid information leaks via compiler padding etc. i could be more wise to do a memset(p_ca_slot_info,0,sizeof (struct ca_slot_info)) and then set the

There is no compiler padding. My static checker looks for that.

regards,
dan carpenter