[PATCH] udev - safer string handling - part three

Here we truncate our input strings from the environment to our
defined limit. It's a bit theroretical but better check for it.

It cleans up some magic length definitions and removes the code
duplication in udev, udevtest and udevsend.

udevd needs to be killed after installation, cause the message size
is changed with this patch.
Should we do this with the 'make install', like we do with the '.udevdb'?

diff --git a/namedev.c b/namedev.c
index f688507dc39c985b389e3b1c68cf2efafce110f3..bc407dd103a614ec90b3c3bfb77115df706a20db 100644 (file)
--- a/namedev.c
+++ b/namedev.c
@@ -405,7 +405,7 @@ static int execute_program(char *path, char *value, int len)
        int fds[2];
        pid_t pid;
        int value_set = 0;
-       char buffer[256];
+       char buffer[255];
        char *pos;
        char *args[PROGRAM_MAXARG];
        int i;

diff --git a/udev.c b/udev.c
index 1c65f2e59c5063753fc92e7c5cb5a134c22ddc1f..4486707f426a68f748cf23133fb1afd24c35232a 100644 (file)
--- a/udev.c
+++ b/udev.c
@@ -43,7 +43,7 @@ char **main_envp;
 unsigned char logname[42];
 void log_message (int level, const char *format, ...)
 {
-       va_list args;
+       va_list args;
 
        if (!udev_log)
                return;
@@ -67,30 +67,6 @@ static void sig_handler(int signum)
        }
 }
 
-static inline char *get_action(void)
-{
-       char *action;
-
-       action = getenv("ACTION");
-       return action;
-}
-
-static inline char *get_devpath(void)
-{
-       char *devpath;
-
-       devpath = getenv("DEVPATH");
-       return devpath;
-}
-
-static inline char *get_seqnum(void)
-{
-       char *seqnum;
-
-       seqnum = getenv("SEQNUM");
-       return seqnum;
-}
-
 static char *subsystem_blacklist[] = {
        "net",
        "scsi_host",
@@ -130,7 +106,7 @@ static int udev_hotplug(int argc, char **argv)
        }
 
        /* skip blacklisted subsystems */
-       subsystem = argv[1];
+       subsystem = get_subsystem(argv[1]);
        if (!subsystem) {
                dbg("no subsystem?");
                goto exit;
@@ -200,5 +176,3 @@ int main(int argc, char **argv, char **envp)
 
        return udev_hotplug(argc, argv);
 }
-
-

diff --git a/udev.h b/udev.h
index 0ce010f3c49071131795f70a0873c02fba3d9428..fa56c366911a511e15fed9b2b5bb8e05633ad994 100644 (file)
--- a/udev.h
+++ b/udev.h
@@ -23,6 +23,8 @@
 #ifndef UDEV_H
 #define UDEV_H
 
+#include <stdlib.h>
+#include <string.h>
 #include <sysfs/libsysfs.h>
 #include <stddef.h>
 #include <sys/param.h>
@@ -34,6 +36,10 @@
 #define GROUP_SIZE     30
 #define MODE_SIZE      8
 
+#define ACTION_SIZE    30
+#define DEVPATH_SIZE   255
+#define SUBSYSTEM_SIZE 30
+
 /* length of public data */
 #define UDEVICE_LEN (offsetof(struct udevice, bus_id))
 
@@ -79,6 +85,45 @@ do { \
        strncat(to, from, maxsize - strlen(to)-1); \
 } while (0)
 
+static inline char *get_action(void)
+{
+       char *action;
+
+       action = getenv("ACTION");
+       if (strlen(action) > ACTION_SIZE)
+               action[ACTION_SIZE-1] = '\0';
+
+       return action;
+}
+
+static inline char *get_devpath(void)
+{
+       char *devpath;
+
+       devpath = getenv("DEVPATH");
+       if (strlen(devpath) > DEVPATH_SIZE)
+               devpath[DEVPATH_SIZE-1] = '\0';
+
+       return devpath;
+}
+
+static inline char *get_seqnum(void)
+{
+       char *seqnum;
+
+       seqnum = getenv("SEQNUM");
+
+       return seqnum;
+}
+
+static inline char *get_subsystem(char *subsystem)
+{
+       if (strlen(subsystem) > SUBSYSTEM_SIZE)
+               subsystem[SUBSYSTEM_SIZE-1] = '\0';
+
+       return subsystem;
+}
+
 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/udevd.c b/udevd.c
index f7901cb7529ca39a3873bf6bd561acd801040194..2b3dc55788f3e2915c7edf6644e7810b7c062dac 100644 (file)
--- a/udevd.c
+++ b/udevd.c
@@ -119,8 +119,8 @@ static void msg_queue_insert(struct hotplug_msg *msg)
 static void udev_run(struct hotplug_msg *msg)
 {
        pid_t pid;
-       char action[32];
-       char devpath[256];
+       char action[ACTION_SIZE];
+       char devpath[DEVPATH_SIZE];
        char *env[] = { action, devpath, NULL };
 
        snprintf(action, sizeof(action), "ACTION=%s", msg->action);

diff --git a/udevd.h b/udevd.h
index 8efe1d569c59f910e65fccf07e2bcebeff14db6e..8b82ff9a7015ae19a85626cc644fc487bc153c21 100644 (file)
--- a/udevd.h
+++ b/udevd.h
@@ -35,7 +35,7 @@ struct hotplug_msg {
        pid_t pid;
        int seqnum;
        time_t queue_time;
-       char action[8];
-       char devpath[128];
-       char subsystem[16];
+       char action[ACTION_SIZE];
+       char devpath[DEVPATH_SIZE];
+       char subsystem[SUBSYSTEM_SIZE];
 };

diff --git a/udevsend.c b/udevsend.c
index 08212dfee62552cfb8c6bb3edc825c75a707e0b9..4b6ef5c2f3b7f8929678d85ecd6524cf07113765 100644 (file)
--- a/udevsend.c
+++ b/udevsend.c
@@ -52,30 +52,6 @@ void log_message (int level, const char *format, ...)
 }
 #endif
 
-static inline char *get_action(void)
-{
-       char *action;
-
-       action = getenv("ACTION");
-       return action;
-}
-
-static inline char *get_devpath(void)
-{
-       char *devpath;
-
-       devpath = getenv("DEVPATH");
-       return devpath;
-}
-
-static inline char *get_seqnum(void)
-{
-       char *seqnum;
-
-       seqnum = getenv("SEQNUM");
-       return seqnum;
-}
-
 static int build_hotplugmsg(struct hotplug_msg *msg, char *action,
                            char *devpath, char *subsystem, int seqnum)
 {
@@ -144,7 +120,7 @@ int main(int argc, char* argv[])
 #endif
        dbg("version %s", UDEV_VERSION);
 
-       subsystem = argv[1];
+       subsystem = get_subsystem(argv[1]);
        if (subsystem == NULL) {
                dbg("no subsystem");
                goto exit;

diff --git a/udevtest.c b/udevtest.c
index 4956758636206343ba0139c6dec8d89da7d7beae..4bc094fd908f8c2f3dd21ea10f287f6de300f6a8 100644 (file)
--- a/udevtest.c
+++ b/udevtest.c
@@ -66,30 +66,6 @@ static void sig_handler(int signum)
        }
 }
 
-static inline char *get_action(void)
-{
-       char *action;
-
-       action = getenv("ACTION");
-       return action;
-}
-
-static inline char *get_devpath(void)
-{
-       char *devpath;
-
-       devpath = getenv("DEVPATH");
-       return devpath;
-}
-
-static inline char *get_seqnum(void)
-{
-       char *seqnum;
-
-       seqnum = getenv("SEQNUM");
-       return seqnum;
-}
-
 static char *subsystem_blacklist[] = {
        "net",
        "scsi_host",