Skip to contentSkip to conversation

vga_switcheroo: comparing too few characters in strncmp()

PATCH In review Dan Carpenter 3 participants 4 messages Jan 7, 2011

Subsystems (4)
DRM drivers DRM misc Kernel janitors VGA_SWITCHEROO
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 vga_switcheroo: comparing too few characters in strncmp()

This is a copy-and-paste bug. We should be comparing 4 characters here instead of 3.

Signed-off-by Dan Carpenter <[email protected]>
drivers/gpu/vga/vga_switcheroo.c +2 -2
index d2d8543..e01cacb 100644
@@ -346,11 +346,11 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
 	if (strncmp(usercmd, "DIS", 3) = 0)
 		client_id = VGA_SWITCHEROO_DIS;
 
-	if (strncmp(usercmd, "MIGD", 3) = 0) {
+	if (strncmp(usercmd, "MIGD", 4) = 0) {
 		just_mux = true;
 		client_id = VGA_SWITCHEROO_IGD;
 	}
-	if (strncmp(usercmd, "MDIS", 3) = 0) {
+	if (strncmp(usercmd, "MDIS", 4) = 0) {
 		just_mux = true;
 		client_id = VGA_SWITCHEROO_DIS;
 	}
#1 lore raw
Quoted 6 lines · Dan Carpenter

This is a copy-and-paste bug. We should be comparing 4 characters here instead of 3.

drivers/gpu/vga/vga_switcheroo.c vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf, · line 353

Quoted 10 lines
drivers/gpu/vga/vga_switcheroo.c +2 -2
@@ -346,11 +346,11 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
 	if (strncmp(usercmd, "DIS", 3) = 0)
 		client_id = VGA_SWITCHEROO_DIS;
 
-	if (strncmp(usercmd, "MIGD", 3) = 0) {
+	if (strncmp(usercmd, "MIGD", 4) = 0) {
 		just_mux = true;
 		client_id = VGA_SWITCHEROO_IGD;
 	}
-	if (strncmp(usercmd, "MDIS", 3) = 0) {
+	if (strncmp(usercmd, "MDIS", 4) = 0) {

How about you NUL-terminate, and use strcmp().

author
#2 lore raw
Quoted 5 lines · Alexey Dobriyan

drivers/gpu/vga/vga_switcheroo.c vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf, · line 353

Quoted 2 lines · Dan Carpenter
drivers/gpu/vga/vga_switcheroo.c +1 -1
-	if (strncmp(usercmd, "MDIS", 3) = 0) {
+	if (strncmp(usercmd, "MDIS", 4) = 0) {

How about you NUL-terminate, and use strcmp().

I assume there is a reason for this userspace API. Dave?

regards,
dan carpenter

#3 lore raw
In reply to Alexey Dobriyan
Quoted 10 lines · Alexey Dobriyan
Quoted 9 lines · Dan Carpenter

This is a copy-and-paste bug. We should be comparing 4 characters here instead of 3.

drivers/gpu/vga/vga_switcheroo.c +0 -0
@@ -346,11 +346,11 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf,
 	if (strncmp(usercmd, "DIS", 3) == 0)
 		client_id = VGA_SWITCHEROO_DIS;
Quoted 9 lines

drivers/gpu/vga/vga_switcheroo.c vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf, · line 353

Quoted 7 lines
drivers/gpu/vga/vga_switcheroo.c +2 -2
-	if (strncmp(usercmd, "MIGD", 3) == 0) {
+	if (strncmp(usercmd, "MIGD", 4) == 0) {
 		just_mux = true;
 		client_id = VGA_SWITCHEROO_IGD;
 	}
-	if (strncmp(usercmd, "MDIS", 3) == 0) {
+	if (strncmp(usercmd, "MDIS", 4) == 0) {

How about you NUL-terminate, and use strcmp().

Userspace data? How to guarantee NULL terminated string at runtime?

Still, the string constants being compared against are already NULL terminated, so he could use strcmp as is.

Also, strcmp() would discard user strings that start with a matching substring ("DIS", "DISK", "DISH", "DISINTERESTED" would all set client_id to VGA_SWITCHEROO_DIS, using strncmp(), but strcmp() would only match "DIS").

Quoted 5 lines

--
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/