Skip to contentSkip to conversation

symlink name and possible overflow in create_node()

PATCH In review Olaf Hering 4 participants 6 messages Feb 17, 2004 – Feb 18, 2004

lore
Share & tools
lore

Olaf Hering

author
#0 lore raw

PATCH 1/1 symlink name and possible overflow in create_node()

the filename array might overflow if I interpret the strncat man page correctly.

Why does a symlink look like './foo' instead of just 'foo'?

udev-add.c 2004-02-17 19:56:56.000000000 +0100 renamed from udev-add.c~ 2004-02-16 22:58:24.000000000 +0100 +1 -3
@@ -146,7 +146,7 @@ static int create_node(struct udevice *d
        int tail;
 
        strncpy(filename, udev_root, sizeof(filename));
-       strncat(filename, dev->name, sizeof(filename));
+       strncat(filename, dev->name, sizeof(filename) - strlen(filename));
 
        switch (dev->type) {
        case 'b':

@@ -247,8 +247,6 @@ static int create_node(struct udevice *d
                                i++;
                        }
 
-                       if (linktarget[0] = '\0')
-                               strcpy(linktarget, "./");
                        strcat(linktarget, &dev->name[tail]);
 
                        /* unlink existing files to ensure that our symlink is created */
Signature · 14 lines
USB is for mice, FireWire is for men!

sUse lINUX ag, nÜRNBERG


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id56&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#1 lore raw

udev-add.c 2004-02-17 19:56:56.000000000 +0100 static int create_node(struct udevice *d · line 152

Quoted 13 lines · Olaf Hering

the filename array might overflow if I interpret the strncat man page correctly.

udev-add.c 2004-02-17 19:56:56.000000000 +0100 renamed from udev-add.c~ 2004-02-16 22:58:24.000000000 +0100 +1 -1
@@ -146,7 +146,7 @@ static int create_node(struct udevice *d
        int tail;
 
        strncpy(filename, udev_root, sizeof(filename));
-       strncat(filename, dev->name, sizeof(filename));
+       strncat(filename, dev->name, sizeof(filename) - strlen(filename));
 
        switch (dev->type) {
        case 'b':

strncpy/strncat don't null terminate strings if it hits the maximum size. Looks like all of those calls need some code explicitly null terminate the strings as well.

	strncpy(filename, udev_root, sizeof(filename));
	filename[sizeof(filename) - 1] = 0;
	strncat(filename, dev->name, sizeof(filename) - strlen(filename));
	filename[sizeof(filename) - 1] = 0;

Otherwise, it could cause strange behaviour when the strings are actually used.

JE

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id56&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#2 lore raw
In reply to Olaf Hering
Quoted 12 lines · Johannes Erdfelt
Quoted 11 lines · Olaf Hering

the filename array might overflow if I interpret the strncat man page correctly.

udev-add.c 2004-02-17 19:56:56.000000000 +0100 renamed from udev-add.c~ 2004-02-16 22:58:24.000000000 +0100 +1 -1
@@ -146,7 +146,7 @@ static int create_node(struct udevice *d
        int tail;
 
        strncpy(filename, udev_root, sizeof(filename));
-       strncat(filename, dev->name, sizeof(filename));
+       strncat(filename, dev->name, sizeof(filename) - strlen(filename));
Quoted 6 lines

udev-add.c 2004-02-17 19:56:56.000000000 +0100 static int create_node(struct udevice *d · line 152

Quoted 2 lines
udev-add.c 2004-02-17 19:56:56.000000000 +0100 renamed from udev-add.c~ 2004-02-16 22:58:24.000000000 +0100 +0 -0
        switch (dev->type) {
        case 'b':

strncpy/strncat don't null terminate strings if it hits the maximum size. Looks like all of those calls need some code explicitly null terminate the strings as well.

Oh, we have strfieldcpy() for this.
Maybe we should have strfieldcat() too.

Anyone, who wants to send a patch? :)

thanks,
Kay

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id56&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#3 lore raw
In reply to Olaf Hering
Quoted 4 lines · Kay Sievers

Oh, we have strfieldcpy() for this.
Maybe we should have strfieldcat() too.

Anyone, who wants to send a patch? :)

Andrey Borzenkov has that function in his include multiple udev.rules patch.

Signature · 5 lines
John
email: [email protected]               Quis custodiet ipsos custodes
Yahoo Messenger: jfjellstad
MSN Messenger: [email protected]
web: http://www.fjellstad.org/
#4 lore raw
In reply to Olaf Hering
Quoted 8 lines · John L. Fjellstad
Quoted 4 lines · Kay Sievers

Oh, we have strfieldcpy() for this.
Maybe we should have strfieldcat() too.

Anyone, who wants to send a patch? :)

Andrey Borzenkov has that function in his include multiple udev.rules patch.

Oh, I meant a careful walk through the code and looking for the string processing issues and not missing a simple function.

And the task is still unassigned :)

thanks,
Kay

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id56&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#5 lore raw
In reply to Olaf Hering
Quoted 4 lines · Kay Sievers

Oh, I meant a careful walk through the code and looking for the string processing issues and not missing a simple function.

And the task is still unassigned :)

I can take a look at it, and see if I can't come up with any (if necessary) by Friday.

Signature · 2 lines
John L. Fjellstad
web: http://www.fjellstad.org/          Quis custodiet ipsos custodes