[multipathd] uxsock patch

I was playing around with the CLI, and having some issues. Occasionally
commands wouldn't work, which was because multipathd was using string
functions on strings that weren't NULL terminated. Also, if you just typed
in junk for the name of the map to remove, it would always remove dm-0,
because atoi returned 0.  I fixed those, added some moron-proofing checks,
and made multipathd reply "fail" if the command failed.

Here's the patch

Benjamin Marzinski, Redhat.

diff --git a/multipathd/main.c b/multipathd/main.c
index 10e9db3..7b3d1c9 100644 (file)
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -210,29 +210,35 @@ out:
        return 1;
 }
 
-static void
+static int
 switch_to_pathgroup (char * str)
 {
        char * mapname;
-       char * buff;
-       char * p;
+       char * buff = NULL;
+       char * p = NULL;
+       int r = 0;
+       int pg;
 
        p = str;
        p += get_word(p, &mapname);
 
        if (!mapname)
-               return;
+               return 1;
 
        p += get_word(p, &buff);
 
        if (!buff)
                goto out;
 
-       dm_switchgroup(mapname, atoi(buff));
+       if (sscanf(buff, "%d", &pg) != 1)
+               goto out1;
+
+       r = !dm_switchgroup(mapname, pg);
+out1:
        FREE(buff);
 out:
        FREE(mapname);
-       return;
+       return r;
 }
        
 static void
@@ -560,13 +566,15 @@ uev_remove_map (char * devname, struct paths * allpaths)
        int minor;
        struct multipath * mpp;
 
-       minor = atoi(devname + 3);
+       if (sscanf(devname, "dm-%d", &minor) != 1)
+               return 1;
+
        mpp = find_mp_by_minor(allpaths->mpvec, minor);
 
        if (!mpp) {
-               condlog(4, "%s: devmap not registered, can't remove",
+               condlog(3, "%s: devmap not registered, can't remove",
                        devname);
-               return 0;
+               return 1;
        }
 
        condlog(2, "remove %s devmap", mpp->alias);
@@ -584,15 +592,17 @@ uev_add_path (char * devname, struct paths * allpaths)
 
        if (pp) {
                condlog(3, "%s: already in pathvec");
-               return 0;
+               return 1;
        }
-       condlog(2, "add %s path checker", devname);
        pp = store_pathinfo(allpaths->pathvec, conf->hwtable,
                       devname, DI_SYSFS | DI_WWID);
 
-       if (!pp)
+       if (!pp) {
+               condlog(0, "%s: failed to store path info", devname);
                return 1;
+       }
 
+       condlog(2, "%s: path checker registered", devname);
        pp->mpp = find_mp_by_wwid(allpaths->mpvec, pp->wwid);
 
        if (pp->mpp)
@@ -614,7 +624,7 @@ uev_remove_path (char * devname, struct paths * allpaths)
 
        if (!pp) {
                condlog(3, "%s: not in pathvec");
-               return 0;
+               return 1;
        }
        condlog(2, "remove %s path checker", devname);
        i = find_slot(allpaths->pathvec, (void *)pp);
@@ -664,6 +674,7 @@ show_paths (struct paths * allpaths)
                c += sprintf(c, "\n");
        }
 
+       reply[MAX_REPLY_LEN - 1] = 0;
        return reply;
 }
 
@@ -705,12 +716,14 @@ show_maps (struct paths * allpaths)
                c += sprintf(c, "\n");
        }
 
+       reply[MAX_REPLY_LEN - 1] = 0;
        return reply;
 }
 
 char *
 uxsock_trigger (char * str, void * trigger_data)
 {
+       int r;
        struct paths * allpaths;
        char * reply = NULL;
 
@@ -718,31 +731,47 @@ uxsock_trigger (char * str, void * trigger_data)
 
        lock(allpaths->lock);
 
-       if (*str == 'l' && *(str + 1) == 'p')
+       if (strlen(str) < 2)
+               r = 1;
+       else if (*str == 'l' && *(str + 1) == 'p')
                reply = show_paths(allpaths);
 
        else if (*str == 'l' && *(str + 1) == 'm')
                reply = show_maps(allpaths);
 
+       else if (strlen(str) < 4)
+               r = 1;
+
        else if (*str == 'r' && *(str + 1) == 'p')
-               uev_remove_path(str + 3, allpaths);
+               r = uev_remove_path(str + 3, allpaths);
 
        else if (*str == 'a' && *(str + 1) == 'p')
-               uev_add_path(str + 3, allpaths);
+               r = uev_add_path(str + 3, allpaths);
 
-       else if (*str == 'r' && *(str + 1) == 'm')
-               if (!strncmp(str +3, "dm-", 3))
-                       uev_remove_map(str + 3, allpaths);
+       else if (*str == 'r' && *(str + 1) == 'm') {
+               if (!strncmp(str +3, "dm-", 3)) {
+                       r = uev_remove_map(str + 3, allpaths);
+               }
+       }
 
-       else if (*str == 'a' && *(str + 1) == 'm')
-               if (!strncmp(str +3, "dm-", 3))
-                       uev_add_map(str + 3, allpaths);
+       else if (*str == 'a' && *(str + 1) == 'm') {
+               if (!strncmp(str +3, "dm-", 3)) {
+                       r = uev_add_map(str + 3, allpaths);
+               }
+       }
 
        else if (*str == 's' && *(str + 1) == 'g')
-               switch_to_pathgroup(str + 3);
+               r = switch_to_pathgroup(str + 3);
 
-       if (!reply)
-               reply = strdup("ok\n");
+       else
+               r = 1;
+
+       if (!reply) {
+               if (r)
+                       reply = strdup("fail\n");
+               else
+                       reply = strdup("ok\n");
+       }
 
        unlock(allpaths->lock);
 

diff --git a/multipathd/uxclnt.c b/multipathd/uxclnt.c
index accc08c..f40b393 100644 (file)
--- a/multipathd/uxclnt.c
+++ b/multipathd/uxclnt.c
@@ -22,12 +22,12 @@ static void process(int fd)
        char *reply;
 
        while ((line = readline("multipathd> "))) {
-               size_t len = strlen(line);
+               size_t len;
 
-               if (send_packet(fd, line, strlen(line)) != 0) break;
+               if (send_packet(fd, line, strlen(line) + 1) != 0) break;
                if (recv_packet(fd, &reply, &len) != 0) break;
 
-               printf("%*.*s\n", (int)len, (int)len, reply);
+               printf("%s\n", reply);
 
                if (line && *line)
                        add_history(line);
@@ -42,10 +42,10 @@ static void process_req(int fd, char * inbuf)
        char *reply;
        size_t len;
 
-       send_packet(fd, inbuf, strlen(inbuf));
+       send_packet(fd, inbuf, strlen(inbuf) + 1);
        recv_packet(fd, &reply, &len);
 
-       printf("%*.*s\n", (int)len, (int)len, reply);
+       printf("%s\n", reply);
        free(reply);
 }
        

diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index faaa2e9..b0ed6f5 100644 (file)
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -120,14 +120,14 @@ void * uxsock_listen(char * (*uxsock_trigger)(char *, void * trigger_data),
                                if (recv_packet(c->fd, &inbuf, &len) != 0) {
                                        dead_client(c);
                                } else {
-                                       condlog(4, "Got request [%*.*s]",
-                                               (int)len, (int)len, inbuf);
+                                       inbuf[len - 1] = 0;
+                                       condlog(4, "Got request [s]", inbuf);
                                        reply = uxsock_trigger(inbuf,
                                                        trigger_data);
 
                                        if (reply) {
                                                if (send_packet(c->fd, reply,
-                                                       strlen(reply)) != 0) {
+                                                    strlen(reply) + 1) != 0) {
                                                        dead_client(c);
                                                }
                                                FREE(reply);