usbip: fix usbip bind writing random string after command in match_busid

PATCH In review Juan Zea 3 participants 7 messages Dec 13, 2017 – Dec 15, 2017

Subsystems (2)

USB USB over IP driver

lore

Share & tools

lore

Juan Zea

author

2017-12-13 10:16 UTC #0 lore raw

usbip bind writes commands followed by random string when writing to match_busid attribute in sysfs, caused by using full variable size instead of string length.

Signed-off-by Juan Zea <[email protected]>

---
 tools/usb/usbip/src/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/usb/usbip/src/utils.c +1 -1

index 2b3d6d2..ea1a1af 100644
@@ -42,7 +42,7 @@ int modify_match_busid(char *busid, int add)
                snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
 
        rc = write_sysfs_attribute(match_busid_attr_path, command,
-                                  sizeof(command));
+                                  strlen(command));
        if (rc < 0) {
                dbg("failed to write match_busid: %s", strerror(errno));
                return -1;

Juan Zea

author

2017-12-13 11:07 UTC #1 lore raw

PATCH 1/1 usbip: fix usbip bind writing random string after command in match_busid

usbip bind writes commands followed by random string when writing to match_busid attribute in sysfs, caused by using full variable size instead of string length.

Signed-off-by Juan Zea <[email protected]>

---
 tools/usb/usbip/src/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/usb/usbip/src/utils.c +1 -1

index 2b3d6d2..ea1a1af 100644
@@ -42,7 +42,7 @@ int modify_match_busid(char *busid, int add)
                snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
 
        rc = write_sysfs_attribute(match_busid_attr_path, command,
-                                  sizeof(command));
+                                  strlen(command));
        if (rc < 0) {
                dbg("failed to write match_busid: %s", strerror(errno));
                return -1;

Shuah Khan

maintainer

2017-12-13 17:42 UTC #2 lore raw

Quoted 15 lines · Juan Zea

usbip bind writes commands followed by random string when writing to match_busid attribute in sysfs, caused by using full variable size instead of string length.
Signed-off-by Juan Zea <[email protected]>
---
 tools/usb/usbip/src/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/usb/usbip/src/utils.c +0 -0
index 2b3d6d2..ea1a1af 100644

tools/usb/usbip/src/utils.c int modify_match_busid(char *busid, int add) · line 48

Quoted 8 lines

tools/usb/usbip/src/utils.c +1 -1

@@ -42,7 +42,7 @@ int modify_match_busid(char *busid, int add)
                snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
 
        rc = write_sysfs_attribute(match_busid_attr_path, command,
-                                  sizeof(command));
+                                  strlen(command));
        if (rc < 0) {
                dbg("failed to write match_busid: %s", strerror(errno));
                return -1;

Why not use the return value from snprintf() for length, instead of calling strlen(command)?

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

Juan Zea

author

2017-12-14 10:23 UTC #3 lore raw

Quoted 1 line

Why not use the return value from snprintf() for length, instead of calling

strlen(command)?

Yes, that makes sense. Something like this?

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

tools/usb/usbip/src/utils.c +6 -3

index 2b3d6d2..3d7b42e 100644
@@ -30,6 +30,7 @@ int modify_match_busid(char *busid, int add)
        char command[SYSFS_BUS_ID_SIZE + 4];
        char match_busid_attr_path[SYSFS_PATH_MAX];
        int rc;
+       int cmd_size;
 
        snprintf(match_busid_attr_path, sizeof(match_busid_attr_path),
                 "%s/%s/%s/%s/%s/%s", SYSFS_MNT_PATH, SYSFS_BUS_NAME,

@@ -37,12 +38,14 @@ int modify_match_busid(char *busid, int add)
                 attr_name);
 
        if (add)
-               snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s", busid);
+               cmd_size = snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s",
+                                   busid);
        else
-               snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
+               cmd_size = snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s",
+                                   busid);
 
        rc = write_sysfs_attribute(match_busid_attr_path, command,
-                                  sizeof(command));
+                                  cmd_size);
        if (rc < 0) {
                dbg("failed to write match_busid: %s", strerror(errno));
                return -1;

Shuah Khan

2017-12-14 20:14 UTC #4 lore raw

Quoted 4 lines · Juan Zea

Quoted 1 line
Why not use the return value from snprintf() for length, instead of calling
strlen(command)?
Yes, that makes sense. Something like this?

Yes

Quoted 33 lines

tools/usb/usbip/src/utils.c +6 -3

index 2b3d6d2..3d7b42e 100644
@@ -30,6 +30,7 @@ int modify_match_busid(char *busid, int add)
        char command[SYSFS_BUS_ID_SIZE + 4];
        char match_busid_attr_path[SYSFS_PATH_MAX];
        int rc;
+       int cmd_size;
 
        snprintf(match_busid_attr_path, sizeof(match_busid_attr_path),
                 "%s/%s/%s/%s/%s/%s", SYSFS_MNT_PATH, SYSFS_BUS_NAME,

@@ -37,12 +38,14 @@ int modify_match_busid(char *busid, int add)
                 attr_name);
 
        if (add)
-               snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s", busid);
+               cmd_size = snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s",
+                                   busid);
        else
-               snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
+               cmd_size = snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s",
+                                   busid);
 
        rc = write_sysfs_attribute(match_busid_attr_path, command,
-                                  sizeof(command));
+                                  cmd_size);
        if (rc < 0) {
                dbg("failed to write match_busid: %s", strerror(errno));
                return -1;

Quoted 3 lines

Regards,
Juan

Juan Zea

author

2017-12-15 09:21 UTC #5 lore raw

PATCH 1/1 usbip: fix usbip bind writing random string after command in match_busid

usbip bind writes commands followed by random string when writing to match_busid attribute in sysfs, caused by using full variable size instead of string length.

Signed-off-by Juan Zea <[email protected]>

---
 tools/usb/usbip/src/utils.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

tools/usb/usbip/src/utils.c +6 -3

index 2b3d6d2..3d7b42e 100644
@@ -30,6 +30,7 @@ int modify_match_busid(char *busid, int add)
 	char command[SYSFS_BUS_ID_SIZE + 4];
 	char match_busid_attr_path[SYSFS_PATH_MAX];
 	int rc;
+	int cmd_size;
 
 	snprintf(match_busid_attr_path, sizeof(match_busid_attr_path),
 		 "%s/%s/%s/%s/%s/%s", SYSFS_MNT_PATH, SYSFS_BUS_NAME,

@@ -37,12 +38,14 @@ int modify_match_busid(char *busid, int add)
 		 attr_name);
 
 	if (add)
-		snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s", busid);
+		cmd_size = snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s",
+				    busid);
 	else
-		snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
+		cmd_size = snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s",
+				    busid);
 
 	rc = write_sysfs_attribute(match_busid_attr_path, command,
-				   sizeof(command));
+				   cmd_size);
 	if (rc < 0) {
 		dbg("failed to write match_busid: %s", strerror(errno));
 		return -1;

Shuah Khan

Acked-by on v1 1/1

2017-12-15 17:39 UTC #6 lore raw

Quoted 30 lines · Juan Zea

usbip bind writes commands followed by random string when writing to match_busid attribute in sysfs, caused by using full variable size instead of string length.

Signed-off-by Juan Zea <[email protected]>

---
 tools/usb/usbip/src/utils.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

tools/usb/usbip/src/utils.c +3 -1

index 2b3d6d2..3d7b42e 100644
@@ -30,6 +30,7 @@ int modify_match_busid(char *busid, int add)
 	char command[SYSFS_BUS_ID_SIZE + 4];
 	char match_busid_attr_path[SYSFS_PATH_MAX];
 	int rc;
+	int cmd_size;
 
 	snprintf(match_busid_attr_path, sizeof(match_busid_attr_path),
 		 "%s/%s/%s/%s/%s/%s", SYSFS_MNT_PATH, SYSFS_BUS_NAME,

@@ -37,12 +38,14 @@ int modify_match_busid(char *busid, int add)
 		 attr_name);
 
 	if (add)
-		snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s", busid);
+		cmd_size = snprintf(command, SYSFS_BUS_ID_SIZE + 4, "add %s",
+				    busid);
 	else

tools/usb/usbip/src/utils.c int modify_match_busid(char *busid, int add) · line 51

Quoted 10 lines

tools/usb/usbip/src/utils.c +3 -2

-		snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s", busid);
+		cmd_size = snprintf(command, SYSFS_BUS_ID_SIZE + 4, "del %s",
+				    busid);
 
 	rc = write_sysfs_attribute(match_busid_attr_path, command,
-				   sizeof(command));
+				   cmd_size);
 	if (rc < 0) {
 		dbg("failed to write match_busid: %s", strerror(errno));
 		return -1;

Thanks for the patch.

Greg, could you please pick this up.

Acked-by Shuah Khan <[email protected]>

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