multipathd: avoid null pointer dereference in LOG_MSG
authorMartin Wilck <mwilck@suse.de>
Wed, 13 Feb 2019 22:55:05 +0000 (16:55 -0600)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Thu, 14 Feb 2019 17:53:05 +0000 (18:53 +0100)
LOG_MSG() will dereference pp->mpp. Commit cb5ec664 added a call to
LOG_MSG() before the check for (!pp->mpp) in check_path.  This can cause
multipathd to crash.  LOG_MSG() should check the fields before dereferencing
them. Make checker_selected() an inline function to allow the compiler
to optimize away the usually redundant test "if (&checker->pp != NULL)".

Also, checker_message() should fail to a generic message if c->cls isn't
set (which means that a checker hasn't been selected).

Fixes: cb5ec664 (multipathd: check_path: improve logging for "unusable
                 path" case)
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
libmultipath/checkers.c
libmultipath/checkers.h
multipathd/main.c

index 848c4c3..f4fdcae 100644 (file)
@@ -261,13 +261,6 @@ int checker_check (struct checker * c, int path_state)
        return r;
 }
 
-int checker_selected(const struct checker *c)
-{
-       if (!c)
-               return 0;
-       return c->cls != NULL;
-}
-
 const char *checker_name(const struct checker *c)
 {
        if (!c || !c->cls)
@@ -295,7 +288,7 @@ const char *checker_message(const struct checker *c)
 {
        int id;
 
-       if (!c || c->msgid < 0 ||
+       if (!c || !c->cls || c->msgid < 0 ||
            (c->msgid >= CHECKER_GENERIC_MSGTABLE_SIZE &&
             c->msgid < CHECKER_FIRST_MSGID))
                goto bad_id;
index b2e8f9a..dab197f 100644 (file)
@@ -129,6 +129,11 @@ struct checker {
                                                you want to stuff data in. */
 };
 
+static inline int checker_selected(const struct checker *c)
+{
+       return c != NULL && c->cls != NULL;
+}
+
 const char *checker_state_name(int);
 int init_checkers(const char *);
 void cleanup_checkers (void);
@@ -142,7 +147,6 @@ void checker_set_fd (struct checker *, int);
 void checker_enable (struct checker *);
 void checker_disable (struct checker *);
 int checker_check (struct checker *, int);
-int checker_selected(const struct checker *);
 int checker_is_sync(const struct checker *);
 const char *checker_name (const struct checker *);
 /*
index 491832b..e101626 100644 (file)
@@ -92,7 +92,8 @@ static int use_watchdog;
 
 #define LOG_MSG(lvl, verb, pp)                                 \
 do {                                                           \
-       if (lvl <= verb) {                                      \
+       if (pp->mpp && checker_selected(&pp->checker) &&        \
+           lvl <= verb) {                                      \
                if (pp->offline)                                \
                        condlog(lvl, "%s: %s - path offline",   \
                                pp->mpp->alias, pp->dev);       \
@@ -2017,7 +2018,8 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
        }
 
        if (newstate == PATH_WILD || newstate == PATH_UNCHECKED) {
-               condlog(2, "%s: unusable path - checker failed", pp->dev);
+               condlog(2, "%s: unusable path (%s) - checker failed",
+                       pp->dev, checker_state_name(newstate));
                LOG_MSG(2, verbosity, pp);
                conf = get_multipath_config();
                pthread_cleanup_push(put_multipath_config, conf);