From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kay Sievers Date: Tue, 24 Feb 2004 22:50:52 +0000 Subject: [PATCH] udev - safer string handling all over the place Message-Id: <20040224225052.GA24733@vrfy.org> MIME-Version: 1 Content-Type: multipart/mixed; boundary="3MwIy2ne0vdjdPXF" List-Id: To: linux-hotplug@vger.kernel.org --3MwIy2ne0vdjdPXF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --3MwIy2ne0vdjdPXF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="02-stringfield.patch" 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); } --3MwIy2ne0vdjdPXF-- ------------------------------------------------------- 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_id=1356&alloc_id=3438&op=click _______________________________________________ Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net Linux-hotplug-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel