[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.

diff --git a/namedev.c b/namedev.c
index 2f9d8f5f67b08892b55b86903a82edbd8ac00e22..219cb8a4b84406b63f7cfcf8917eb08fcb9eee66 100644 (file)
--- 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 013878c67933bfa80669919c2b8272a0421109a0..d300b0907d82f1aff214abab492e6f7f2ea258da 100644 (file)
--- 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 736316629f9f9f2cba92782f070626743d41d195..0d3131300ff9d175373d79d897493b7c3d376381 100644 (file)
--- 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 c20c651dc53687056d701c0a8714a7f5d39f3b9b..8794429635bd0caec5f53b294dcd713464f720b2 100644 (file)
--- 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 b5088b90c69e136d84e6ecd23dc165829e211e5c..fc44a9847c09f71e4a83e9401be9b9b9f0edbb77 100644 (file)
--- 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 da633a31a1abe3718b984220b820f3b9081aa786..7b672ef363e33454192dc635d8f667a86dffb0c3 100644 (file)
--- 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 f9fca4ab70abf66c7fab39788420ea04a400658d..a1f79a7c65b016b39bc0b0b295d8ef107252ecdd 100644 (file)
--- 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 b94376c9cee22131d7348f4f837982efc7bbe021..defed2ee31a96e38aeaae3d0346323d2af778097 100644 (file)
--- 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 b5850294aef18aa1cf7fa42fe5456c783bd9d25f..08212dfee62552cfb8c6bb3edc825c75a707e0b9 100644 (file)
--- 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);
 }