Skip to contentSkip to conversation

media: info leak in media_device_enum_entities()

PATCH In review Dan Carpenter 2 participants 3 messages Apr 21, 2013

Subsystems (2)
Media (V4L/DVB) Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 media: info leak in media_device_enum_entities()

The last part of the "u_ent.name" buffer isn't cleared so it still has uninitialized stack memory.

Signed-off-by Dan Carpenter <[email protected]>
drivers/media/media-device.c +6 -3
index 99b80b6..1957c0d 100644
@@ -102,9 +102,12 @@ static long media_device_enum_entities(struct media_device *mdev,
 		return -EINVAL;
 
 	u_ent.id = ent->id;
-	u_ent.name[0] = '\0';
-	if (ent->name)
-		strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
+	if (ent->name) {
+		strncpy(u_ent.name, ent->name, sizeof(u_ent.name));
+		u_ent.name[sizeof(u_ent.name) - 1] = '\0';
+	} else {
+		memset(u_ent.name, 0, sizeof(u_ent.name));
+	}
 	u_ent.type = ent->type;
 	u_ent.revision = ent->revision;
 	u_ent.flags = ent->flags;
#1 lore raw

Am 21.04.2013 13:10, schrieb Dan Carpenter:

Quoted 12 lines

The last part of the "u_ent.name" buffer isn't cleared so it still has uninitialized stack memory.

Signed-off-by Dan Carpenter <[email protected]>
drivers/media/media-device.c +0 -0
index 99b80b6..1957c0d 100644
@@ -102,9 +102,12 @@ static long media_device_enum_entities(struct media_device *mdev,
 		return -EINVAL;

drivers/media/media-device.c static long media_device_enum_entities(struct media_device *mdev, · line 110

Quoted 10 lines
drivers/media/media-device.c +6 -3
 	u_ent.id = ent->id;
-	u_ent.name[0] = '\0';
-	if (ent->name)
-		strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
+	if (ent->name) {
+		strncpy(u_ent.name, ent->name, sizeof(u_ent.name));
+		u_ent.name[sizeof(u_ent.name) - 1] = '\0';
+	} else {
+		memset(u_ent.name, 0, sizeof(u_ent.name));
+	}

I would always memset()
and then do strncpy() for sizeof(u_ent.name) - 1
the rest is always zero.

re,
wh

Quoted 8 lines
drivers/media/media-device.c +0 -1
 	u_ent.type = ent->type;
 	u_ent.revision = ent->revision;
 	u_ent.flags = ent->flags;
--

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

author
#2 lore raw
Quoted 15 lines · walter harms

Am 21.04.2013 13:10, schrieb Dan Carpenter:

Quoted 12 lines

The last part of the "u_ent.name" buffer isn't cleared so it still has uninitialized stack memory.

Signed-off-by Dan Carpenter <[email protected]>
drivers/media/media-device.c +0 -0
index 99b80b6..1957c0d 100644
@@ -102,9 +102,12 @@ static long media_device_enum_entities(struct media_device *mdev,
 		return -EINVAL;
Quoted 14 lines

drivers/media/media-device.c static long media_device_enum_entities(struct media_device *mdev, · line 110

Quoted 10 lines
drivers/media/media-device.c +6 -3
 	u_ent.id = ent->id;
-	u_ent.name[0] = '\0';
-	if (ent->name)
-		strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
+	if (ent->name) {
+		strncpy(u_ent.name, ent->name, sizeof(u_ent.name));
+		u_ent.name[sizeof(u_ent.name) - 1] = '\0';
+	} else {
+		memset(u_ent.name, 0, sizeof(u_ent.name));
+	}

I would always memset()
and then do strncpy() for sizeof(u_ent.name) - 1
the rest is always zero.

Both ways are fine. You'd still have to test for "if (ent->name)", of course. This way is a little faster because I do the test first.

Mauro, if you want I can redo it?

regards,
dan carpenter