Skip to contentSkip to conversation

9p: strlen() doesn't count the terminator

PATCH In review Dan Carpenter 3 participants 4 messages Jul 10, 2010 – Jul 13, 2010

Subsystems (4)
Networking Network drivers Kernel janitors 9P file system
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 9p: strlen() doesn't count the terminator

This is an off by one bug because strlen() doesn't count the NULL terminator. We strcpy() addr into a fixed length array of size UNIX_PATH_MAX later on.

The addr variable is the name of the device being mounted.

CC [email protected]
Signed-off-by Dan Carpenter <[email protected]>
net/9p/trans_fd.c +1 -1
index 98ce9bc..c85109d 100644
@@ -948,7 +948,7 @@ p9_fd_create_unix(struct p9_client *client, const char *addr, char *args)
 
 	csocket = NULL;
 
-	if (strlen(addr) > UNIX_PATH_MAX) {
+	if (strlen(addr) >= UNIX_PATH_MAX) {
 		P9_EPRINTK(KERN_ERR, "p9_trans_unix: address too long: %s\n",
 			addr);
 		return -ENAMETOOLONG;
#1 lore raw
Quoted 14 lines · On Sat, 10 Jul 2010 11:51:54 +0200 Dan Carpenter <[email protected]> wrote:

This is an off by one bug because strlen() doesn't count the NULL terminator. We strcpy() addr into a fixed length array of size UNIX_PATH_MAX later on.

The addr variable is the name of the device being mounted.

CC [email protected]
Signed-off-by Dan Carpenter <[email protected]>
net/9p/trans_fd.c +0 -0
index 98ce9bc..c85109d 100644

net/9p/trans_fd.c p9_fd_create_unix(struct p9_client *client, const char *addr, char *args) · line 954

Quoted 8 lines
net/9p/trans_fd.c +1 -1
@@ -948,7 +948,7 @@ p9_fd_create_unix(struct p9_client *client, const char *addr, char *args)
 
 	csocket = NULL;
 
-	if (strlen(addr) > UNIX_PATH_MAX) {
+	if (strlen(addr) >= UNIX_PATH_MAX) {
 		P9_EPRINTK(KERN_ERR, "p9_trans_unix: address too long: %s\n",
 			addr);
 		return -ENAMETOOLONG;

This bug doesn't strike me as serious enough to warrant backporting the fix into -stable. What was your thinking there?

author
#2 lore raw
Quoted 8 lines · Andrew Morton
Quoted 8 lines
net/9p/trans_fd.c +0 -0
index 98ce9bc..c85109d 100644
@@ -948,7 +948,7 @@ p9_fd_create_unix(struct p9_client *client, const char *addr, char *args)
 
 	csocket = NULL;
Quoted 8 lines

net/9p/trans_fd.c p9_fd_create_unix(struct p9_client *client, const char *addr, char *args) · line 954

Quoted 5 lines
net/9p/trans_fd.c +1 -1
-	if (strlen(addr) > UNIX_PATH_MAX) {
+	if (strlen(addr) >= UNIX_PATH_MAX) {
 		P9_EPRINTK(KERN_ERR, "p9_trans_unix: address too long: %s\n",
 			addr);
 		return -ENAMETOOLONG;

This bug doesn't strike me as serious enough to warrant backporting the fix into -stable. What was your thinking there?

I don't feel strongly about it. It's safe enough and it applies cleanly. On the other hand, root should always control the name of the device to mount so it's not a big deal.

regards,
dan carpenter

maintainer
#3 lore raw
In reply to Andrew Morton

From: Andrew Morton <[email protected]>
Date: Mon, 12 Jul 2010 13:04:58 -0700

Quoted 2 lines

This bug doesn't strike me as serious enough to warrant backporting the fix into -stable. What was your thinking there?

Meanwhile I'll queue this up to net-next-2.6, thanks.