linux-hotplug.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] udev - safer string handling all over the place
@ 2004-02-24 22:50 Kay Sievers
  2004-02-24 23:10 ` Kay Sievers
  2004-02-26 20:56 ` Greg KH
  0 siblings, 2 replies; 3+ messages in thread
From: Kay Sievers @ 2004-02-24 22:50 UTC (permalink / raw)
  To: linux-hotplug

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

Here is the first step towards a safer string handling.
More will follow, but for now only the easy ones :)

Thanks to all who pointed this out. strncat() isn't a nice function. We
all should remember that the destination string is not terminated if the
given lenght is shorter than the strlen of the source string.

And shame on the various implementers of strfieldcat() I found in the
unapplied patches on this list, it's not really better than strncpy()
and hides the real problem.

thanks,
Kay

[-- Attachment #2: 02-stringfield.patch --]
[-- Type: text/plain, Size: 7686 bytes --]

diff -Nru a/namedev_parse.c b/namedev_parse.c
--- a/namedev_parse.c	Tue Feb 24 23:47:05 2004
+++ b/namedev_parse.c	Tue Feb 24 23:47:05 2004
@@ -319,21 +319,21 @@
 			dbg("cannot parse line '%s'", line);
 			continue;
 		}
-		strncpy(dev.name, temp2, sizeof(dev.name));
+		strfieldcpy(dev.name, temp2);
 
 		temp2 = strsep(&temp, ":");
 		if (!temp2) {
 			dbg("cannot parse line '%s'", line);
 			continue;
 		}
-		strncpy(dev.owner, temp2, sizeof(dev.owner));
+		strfieldcpy(dev.owner, temp2);
 
 		temp2 = strsep(&temp, ":");
 		if (!temp2) {
 			dbg("cannot parse line '%s'", line);
 			continue;
 		}
-		strncpy(dev.group, temp2, sizeof(dev.group));
+		strfieldcpy(dev.group, temp2);
 
 		if (!temp) {
 			dbg("cannot parse line: %s", line);
@@ -422,7 +422,7 @@
 	/* parse every file in the list */
 	list_for_each_entry_safe(loop_file, tmp_file, &file_list, list) {
 		strfieldcpy(file, filename);
-		strcat(file, loop_file->name);
+		strfieldcat(file, loop_file->name);
 		parser(file);
 		list_del(&loop_file->list);
 		free(loop_file);
diff -Nru a/udev-add.c b/udev-add.c
--- a/udev-add.c	Tue Feb 24 23:47:05 2004
+++ b/udev-add.c	Tue Feb 24 23:47:05 2004
@@ -78,7 +78,7 @@
 	int retval;
 	struct stat stats;
 	
-	strncpy(p, file, sizeof(p));
+	strfieldcpy(p, file);
 	pos = strchr(p+1, '/');
 	while (1) {
 		pos = strchr(pos+1, '/');
@@ -145,8 +145,8 @@
 	int i;
 	int tail;
 
-	strncpy(filename, udev_root, sizeof(filename));
-	strncat(filename, dev->name, sizeof(filename));
+	strfieldcpy(filename, udev_root);
+	strfieldcat(filename, dev->name);
 
 	switch (dev->type) {
 	case 'b':
@@ -225,8 +225,8 @@
 			if (linkname == NULL || linkname[0] == '\0')
 				break;
 
-			strncpy(filename, udev_root, sizeof(filename));
-			strncat(filename, linkname, sizeof(filename));
+			strfieldcpy(filename, udev_root);
+			strfieldcat(filename, linkname);
 			dbg("symlink '%s' to node '%s' requested", filename, dev->name);
 			if (!fake)
 				if (strrchr(linkname, '/'))
@@ -243,13 +243,13 @@
 			}
 			while (linkname[i] != '\0') {
 				if (linkname[i] == '/')
-					strcat(linktarget, "../");
+					strfieldcat(linktarget, "../");
 				i++;
 			}
 
 			if (linktarget[0] == '\0')
-				strcpy(linktarget, "./");
-			strcat(linktarget, &dev->name[tail]);
+				strfieldcpy(linktarget, "./");
+			strfieldcat(linktarget, &dev->name[tail]);
 
 			/* unlink existing files to ensure that our symlink is created */
 			if (!fake && (lstat(filename, &stats) == 0)) {
@@ -278,8 +278,8 @@
 	char dev_path[SYSFS_PATH_MAX];
 	struct sysfs_class_device *class_dev = NULL;
 
-	strcpy(dev_path, sysfs_path);
-	strcat(dev_path, device_name);
+	strfieldcpy(dev_path, sysfs_path);
+	strfieldcat(dev_path, device_name);
 	dbg("looking at '%s'", dev_path);
 
 	/* open up the sysfs class device for this thing... */
@@ -304,9 +304,9 @@
 	int loop = SECONDS_TO_WAIT_FOR_DEV;
 	int retval;
 
-	strcpy(filename, sysfs_path);
-	strcat(filename, path);
-	strcat(filename, "/dev");
+	strfieldcpy(filename, sysfs_path);
+	strfieldcat(filename, path);
+	strfieldcat(filename, "/dev");
 
 	while (loop--) {
 		struct stat buf;
diff -Nru a/udev-remove.c b/udev-remove.c
--- a/udev-remove.c	Tue Feb 24 23:47:05 2004
+++ b/udev-remove.c	Tue Feb 24 23:47:05 2004
@@ -72,8 +72,8 @@
 	int retval;
 	int i;
 
-	strncpy(filename, udev_root, sizeof(filename));
-	strncat(filename, dev->name, sizeof(filename));
+	strfieldcpy(filename, udev_root);
+	strfieldcat(filename, dev->name);
 
 	info("removing device node '%s'", filename);
 	retval = unlink(filename);
@@ -103,8 +103,8 @@
 			if (linkname == NULL)
 				break;
 
-			strncpy(filename, udev_root, sizeof(filename));
-			strncat(filename, linkname, sizeof(filename));
+			strfieldcpy(filename, udev_root);
+			strfieldcat(filename, linkname);
 
 			dbg("unlinking symlink '%s'", filename);
 			retval = unlink(filename);
@@ -141,7 +141,7 @@
 		temp = strrchr(path, '/');
 		if (temp == NULL)
 			return -ENODEV;
-		strncpy(dev.name, &temp[1], sizeof(dev.name));
+		strfieldcpy(dev.name, &temp[1]);
 	}
 
 	dbg("name is '%s'", dev.name);
diff -Nru a/udev.h b/udev.h
--- a/udev.h	Tue Feb 24 23:47:05 2004
+++ b/udev.h	Tue Feb 24 23:47:05 2004
@@ -61,6 +61,12 @@
 	strncpy(to, from, sizeof(to)-1); \
 } while (0)
 
+#define strfieldcat(to, from) \
+do { \
+	to[sizeof(to)-1] = '\0'; \
+	strncat(to, from, sizeof(to) - strlen(to) -1); \
+} while (0)
+
 extern int udev_add_device(char *path, char *subsystem, int fake);
 extern int udev_remove_device(char *path, char *subsystem);
 extern void udev_init_config(void);
diff -Nru a/udev_dbus.c b/udev_dbus.c
--- a/udev_dbus.c	Tue Feb 24 23:47:05 2004
+++ b/udev_dbus.c	Tue Feb 24 23:47:05 2004
@@ -80,8 +80,8 @@
 	if (sysbus_connection == NULL)
 		return;
 
-	strncpy(filename, udev_root, sizeof(filename));
-	strncat(filename, dev->name, sizeof(filename));
+	strfieldcpy(filename, udev_root);
+	strfieldcat(filename, dev->name);
 
 	/* object, interface, member */
 	message = dbus_message_new_signal("/org/kernel/udev/NodeMonitor", 
@@ -114,8 +114,8 @@
 	if (sysbus_connection == NULL)
 		return;
 
-	strncpy(filename, udev_root, sizeof(filename));
-	strncat(filename, name, sizeof(filename));
+	strfieldcpy(filename, udev_root);
+	strfieldcat(filename, name);
 
 	/* object, interface, member */
 	message = dbus_message_new_signal("/org/kernel/udev/NodeMonitor", 
diff -Nru a/udevdb.c b/udevdb.c
--- a/udevdb.c	Tue Feb 24 23:47:05 2004
+++ b/udevdb.c	Tue Feb 24 23:47:05 2004
@@ -53,7 +53,7 @@
 		return -ENODEV;
 
 	memset(keystr, 0, NAME_SIZE);
-	strcpy(keystr, path);
+	strfieldcpy(keystr, path);
 	key.dptr = keystr;
 	key.dsize = strlen(keystr) + 1;
 
@@ -91,7 +91,7 @@
 		return -EINVAL;
 
 	memset(keystr, 0, sizeof(keystr));
-	strcpy(keystr, path);
+	strfieldcpy(keystr, path);
 
 	key.dptr = keystr;
 	key.dsize = strlen(keystr) + 1;
@@ -180,7 +180,7 @@
 {
 	if (strncmp(dev->name, find_name, sizeof(dev->name)) == 0) {
 		memcpy(find_dev, dev, sizeof(*find_dev));
-		strncpy(find_path, path, NAME_SIZE);
+		strfieldcpy(find_path, path);
 		find_found = 1;
 		/* stop search */
 		return 1;
diff -Nru a/udevinfo.c b/udevinfo.c
--- a/udevinfo.c	Tue Feb 24 23:47:05 2004
+++ b/udevinfo.c	Tue Feb 24 23:47:05 2004
@@ -73,7 +73,7 @@
 
 	dlist_for_each_data(attributes, attr, struct sysfs_attribute) {
 		if (attr->value != NULL) {
-			strncpy(value, attr->value, SYSFS_VALUE_MAX);
+			strfieldcpy(value, attr->value);
 			len = strlen(value);
 			if (len == 0)
 				continue;
@@ -306,8 +306,8 @@
 			} else {
 				if (path[0] != '/') {
 					/* prepend '/' if missing */
-					strcat(temp, "/");
-					strncat(temp, path, sizeof(path));
+					strfieldcat(temp, "/");
+					strfieldcat(temp, path);
 					pos = temp;
 				} else {
 					pos = path;
@@ -343,7 +343,7 @@
 		case NAME:
 			if (root)
 				strfieldcpy(result, udev_root);
-			strncat(result, dev.name, sizeof(result));
+			strfieldcat(result, dev.name);
 			break;
 
 		case SYMLINK:
@@ -385,7 +385,7 @@
 				/* prepend sysfs mountpoint if not given */
 				strfieldcpy(temp, path);
 				strfieldcpy(path, sysfs_path);
-				strncat(path, temp, sizeof(path));
+				strfieldcat(path, temp);
 			}
 			print_device_chain(path);
 			return 0;
diff -Nru a/udevsend.c b/udevsend.c
--- a/udevsend.c	Tue Feb 24 23:47:05 2004
+++ b/udevsend.c	Tue Feb 24 23:47:05 2004
@@ -82,9 +82,9 @@
 	memset(msg, 0x00, sizeof(*msg));
 	strfieldcpy(msg->magic, UDEV_MAGIC);
 	msg->seqnum = seqnum;
-	strncpy(msg->action, action, 8);
-	strncpy(msg->devpath, devpath, 128);
-	strncpy(msg->subsystem, subsystem, 16);
+	strfieldcpy(msg->action, action);
+	strfieldcpy(msg->devpath, devpath);
+	strfieldcpy(msg->subsystem, subsystem);
 	return sizeof(struct hotplug_msg);
 }
 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2004-02-26 20:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-02-24 22:50 [PATCH] udev - safer string handling all over the place Kay Sievers
2004-02-24 23:10 ` Kay Sievers
2004-02-26 20:56 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).