From c472e3c89b9aaad90ad6398c0d2ff5dcf5a9d238 Mon Sep 17 00:00:00 2001 From: "kay.sievers@vrfy.org" Date: Thu, 26 Feb 2004 19:37:47 -0800 Subject: [PATCH] [PATCH] udev - safer string handling all over the place On Tue, Feb 24, 2004 at 11:50:52PM +0100, Kay Sievers wrote: > 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. Hmm, bk didn't checked in one file, maybe I edited it again as root. Nevermind, here is the more complete version. --- namedev.c | 10 +++++----- namedev_parse.c | 8 ++++---- udev-add.c | 26 +++++++++++++------------- udev-remove.c | 10 +++++----- udev.h | 6 ++++++ udev_dbus.c | 8 ++++---- udevdb.c | 6 +++--- udevinfo.c | 10 +++++----- udevsend.c | 6 +++--- 9 files changed, 48 insertions(+), 42 deletions(-) diff --git a/namedev.c b/namedev.c index 2f9d8f5f..219cb8a4 100644 --- a/namedev.c +++ b/namedev.c @@ -157,7 +157,7 @@ static mode_t get_default_mode(void) static char *get_default_owner(void) { if (strlen(default_owner_str) == 0) - strncpy(default_owner_str, "root", OWNER_SIZE); + strfieldcpy(default_owner_str, "root"); return default_owner_str; } @@ -165,7 +165,7 @@ static char *get_default_owner(void) static char *get_default_group(void) { if (strlen(default_group_str) == 0) - strncpy(default_group_str, "root", GROUP_SIZE); + strfieldcpy(default_group_str, "root"); return default_group_str; } @@ -276,7 +276,7 @@ static void apply_format(struct udevice *udev, unsigned char *string, struct sys if (attr != NULL) i = atoi(attr); if (i > 0) { - strncpy(temp1, udev->program_result, sizeof(temp1)); + strfieldcpy(temp1, udev->program_result); pos2 = temp1; while (i) { i--; @@ -837,8 +837,8 @@ done: } else { /* no matching perms found :( */ udev->mode = get_default_mode(); - strncpy(udev->owner, get_default_owner(), OWNER_SIZE); - strncpy(udev->group, get_default_group(), GROUP_SIZE); + strfieldcpy(udev->owner, get_default_owner()); + strfieldcpy(udev->group, get_default_group()); } dbg("name, '%s' is going to have owner='%s', group='%s', mode = %#o", udev->name, udev->owner, udev->group, udev->mode); diff --git a/namedev_parse.c b/namedev_parse.c index 013878c6..d300b090 100644 --- a/namedev_parse.c +++ b/namedev_parse.c @@ -319,21 +319,21 @@ static int namedev_parse_permissions(char *filename) 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 @@ static int call_foreach_file(int parser (char *f) , char *filename, char *extens /* 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 --git a/udev-add.c b/udev-add.c index 73631662..0d313130 100644 --- a/udev-add.c +++ b/udev-add.c @@ -78,7 +78,7 @@ static int create_path(char *file) 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 @@ static int create_node(struct udevice *dev, int fake) 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 @@ static int create_node(struct udevice *dev, int fake) 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 @@ static int create_node(struct udevice *dev, int fake) } 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 @@ static struct sysfs_class_device *get_class_dev(char *device_name) 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 @@ static int sleep_for_dev(char *path) 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 --git a/udev-remove.c b/udev-remove.c index c20c651d..87944296 100644 --- a/udev-remove.c +++ b/udev-remove.c @@ -72,8 +72,8 @@ static int delete_node(struct udevice *dev) 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 @@ static int delete_node(struct udevice *dev) 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 @@ int udev_remove_device(char *path, char *subsystem) 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 --git a/udev.h b/udev.h index b5088b90..fc44a984 100644 --- a/udev.h +++ b/udev.h @@ -61,6 +61,12 @@ do { \ 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 --git a/udev_dbus.c b/udev_dbus.c index da633a31..7b672ef3 100644 --- a/udev_dbus.c +++ b/udev_dbus.c @@ -80,8 +80,8 @@ void sysbus_send_create(struct udevice *dev, const char *path) 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 @@ void sysbus_send_remove(const char* name, const char *path) 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 --git a/udevdb.c b/udevdb.c index f9fca4ab..a1f79a7c 100644 --- a/udevdb.c +++ b/udevdb.c @@ -53,7 +53,7 @@ int udevdb_add_dev(const char *path, const struct udevice *dev) 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 @@ int udevdb_delete_dev(const char *path) 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 @@ static int find_device_by_name(char *path, struct udevice *dev) { 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 --git a/udevinfo.c b/udevinfo.c index b94376c9..defed2ee 100644 --- a/udevinfo.c +++ b/udevinfo.c @@ -73,7 +73,7 @@ static int print_all_attributes(const char *path) 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 @@ static int process_options(void) } 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 @@ print: 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 @@ exit: /* 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 --git a/udevsend.c b/udevsend.c index b5850294..08212dfe 100644 --- a/udevsend.c +++ b/udevsend.c @@ -82,9 +82,9 @@ static int build_hotplugmsg(struct hotplug_msg *msg, char *action, 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); } -- 2.39.5