Skip to contentSkip to conversation

gru: info leak in gru_get_config_info()

PATCH Superseded Dan Carpenter 4 participants 6 messages Apr 21, 2013 – Apr 22, 2013

Subsystems (2)
Kernel janitors Char and misc drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 gru: info leak in gru_get_config_info()

The "info.fill" array isn't initialized so it can leak uninitialized stack information to user space.

Signed-off-by Dan Carpenter <[email protected]>
drivers/misc/sgi-gru/grufile.c +1 -0
index 44d273c..ed5fc43 100644
@@ -176,6 +176,7 @@ static long gru_get_config_info(unsigned long arg)
 	info.nodes = num_online_nodes();
 	info.blades = info.nodes / nodesperblade;
 	info.chiplets = GRU_CHIPLETS_PER_BLADE * info.blades;
+	memset(&info.fill, 0, sizeof(info.fill));
 
 	if (copy_to_user((void __user *)arg, &info, sizeof(info)))
 		return -EFAULT;
#1 lore raw

Am 21.04.2013 13:10, schrieb Dan Carpenter:

Quoted 10 lines

The "info.fill" array isn't initialized so it can leak uninitialized stack information to user space.

Signed-off-by Dan Carpenter <[email protected]>
drivers/misc/sgi-gru/grufile.c +0 -0
index 44d273c..ed5fc43 100644

drivers/misc/sgi-gru/grufile.c static long gru_get_config_info(unsigned long arg) · line 180

Quoted 5 lines
drivers/misc/sgi-gru/grufile.c +1 -0
@@ -176,6 +176,7 @@ static long gru_get_config_info(unsigned long arg)
 	info.nodes = num_online_nodes();
 	info.blades = info.nodes / nodesperblade;
 	info.chiplets = GRU_CHIPLETS_PER_BLADE * info.blades;
+	memset(&info.fill, 0, sizeof(info.fill));

the other way around (clear first all bytes) looks more easy in case someone will add more elements to the struct.

memset(&info, 0, sizeof(info));
info.nodes = num_online_nodes();
info.blades = info.nodes / nodesperblade;
....

re,
wh

Quoted 7 lines
drivers/misc/sgi-gru/grufile.c +0 -1
 	if (copy_to_user((void __user *)arg, &info, sizeof(info)))
 		return -EFAULT;
--

To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
Quoted 22 lines · walter harms

Am 21.04.2013 13:10, schrieb Dan Carpenter:

Quoted 10 lines

The "info.fill" array isn't initialized so it can leak uninitialized stack information to user space.

Signed-off-by Dan Carpenter <[email protected]>
drivers/misc/sgi-gru/grufile.c +0 -0
index 44d273c..ed5fc43 100644

drivers/misc/sgi-gru/grufile.c static long gru_get_config_info(unsigned long arg) · line 180

Quoted 5 lines
drivers/misc/sgi-gru/grufile.c +1 -0
@@ -176,6 +176,7 @@ static long gru_get_config_info(unsigned long arg)
 	info.nodes = num_online_nodes();
 	info.blades = info.nodes / nodesperblade;
 	info.chiplets = GRU_CHIPLETS_PER_BLADE * info.blades;
+	memset(&info.fill, 0, sizeof(info.fill));

the other way around (clear first all bytes) looks more easy in case someone will add more elements to the struct.

Quoted 3 lines

memset(&info, 0, sizeof(info));
info.nodes = num_online_nodes();
info.blades = info.nodes / nodesperblade;

That does seem more safe.

Robin

author
#3 lore raw

PATCH v2 1/1 gru: info leak in gru_get_config_info()

The "info.fill" array isn't initialized so it can leak uninitialized stack information to user space.

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

---
v2: style changes

drivers/misc/sgi-gru/grufile.c +1 -0
index 44d273c..0535d1e 100644
@@ -172,6 +172,7 @@ static long gru_get_config_info(unsigned long arg)
 		nodesperblade = 2;
 	else
 		nodesperblade = 1;
+	memset(&info, 0, sizeof(info));
 	info.cpus = num_online_cpus();
 	info.nodes = num_online_nodes();
 	info.blades = info.nodes / nodesperblade;
Acked-by on v2 1/1
#4 lore raw
Acked-by Dimitri Sivanich <[email protected]>
Quoted 19 lines · Dan Carpenter

The "info.fill" array isn't initialized so it can leak uninitialized stack information to user space.

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

---
v2: style changes

drivers/misc/sgi-gru/grufile.c +1 -0
index 44d273c..0535d1e 100644
@@ -172,6 +172,7 @@ static long gru_get_config_info(unsigned long arg)
 		nodesperblade = 2;
 	else
 		nodesperblade = 1;
+	memset(&info, 0, sizeof(info));
 	info.cpus = num_online_cpus();
 	info.nodes = num_online_nodes();
 	info.blades = info.nodes / nodesperblade;
Acked-by
#5 lore raw
Acked-by Robin Holt <[email protected]>
Quoted 27 lines · Dimitri Sivanich
Acked-by Dimitri Sivanich <[email protected]>
Quoted 12 lines · Dan Carpenter

The "info.fill" array isn't initialized so it can leak uninitialized stack information to user space.

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

---
v2: style changes

drivers/misc/sgi-gru/grufile.c +0 -0
index 44d273c..0535d1e 100644

drivers/misc/sgi-gru/grufile.c static long gru_get_config_info(unsigned long arg) · line 178

Quoted 7 lines
drivers/misc/sgi-gru/grufile.c +1 -0
@@ -172,6 +172,7 @@ static long gru_get_config_info(unsigned long arg)
 		nodesperblade = 2;
 	else
 		nodesperblade = 1;
+	memset(&info, 0, sizeof(info));
 	info.cpus = num_online_cpus();
 	info.nodes = num_online_nodes();
 	info.blades = info.nodes / nodesperblade;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/