Whamcloud - gitweb
LU-10264 mdc: fix possible NULL pointer dereference 21/31621/3
authorAndreas Dilger <andreas.dilger@intel.com>
Fri, 9 Mar 2018 23:18:53 +0000 (16:18 -0700)
committerOleg Drokin <oleg.drokin@intel.com>
Mon, 9 Apr 2018 19:45:47 +0000 (19:45 +0000)
Fix two static analysis errors.

lustre/mdc/mdc_dev.c: in mdc_enqueue_send(), pointer 'matched' return
    from call to function 'ldlm_handle2lock' at line 704 may be NULL
    and will be dereferenced at line 705.
If client is evicted between ldlm_lock_match() and ldlm_handle2lock()
the lock pointer could be NULL.

lustre/lov/lov_dev.c:488 in lov_process_config, sscanf format
    specification '%d' expects type 'int' for 'd', but parameter 3
    has a different type '__u32'.
Converting to kstrtou32() requires changing the "index" variable type
from __u32 to u32, which is fine since it is only used internally,
fix up the few functions that are also passing "__u32 index" and the
resulting checkpatch.pl warnings.

Test-Parameters: trivial
Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
Change-Id: I3cc80d66bbb537161a561f4f2ba7830ddebcab07
Reviewed-on: https://review.whamcloud.com/31621
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Dmitry Eremin <dmitry.eremin@intel.com>
Reviewed-by: Bob Glossman <bob.glossman@intel.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
lustre/lov/lov_dev.c
lustre/lov/lov_internal.h
lustre/lov/lov_obd.c
lustre/mdc/mdc_dev.c

index 40f333b..acd868a 100644 (file)
@@ -447,7 +447,7 @@ static int lov_process_config(const struct lu_env *env,
        int cmd;
        int rc;
        int gen;
-       __u32 index;
+       u32 index;
 
        obd_getref(obd);
 
@@ -479,8 +479,9 @@ static int lov_process_config(const struct lu_env *env,
 
                obd_str2uuid(&tgt_uuid, lustre_cfg_buf(cfg, 1));
 
-               if (sscanf(lustre_cfg_buf(cfg, 2), "%d", &index) != 1)
-                       GOTO(out, rc = -EINVAL);
+               rc = kstrtou32(lustre_cfg_buf(cfg, 2), 10, &index);
+               if (rc)
+                       GOTO(out, rc);
 
                mdc = class_find_client_obd(&tgt_uuid, LUSTRE_MDC_NAME,
                                            &obd->obd_uuid);
index bba4a8d..54c07a8 100644 (file)
@@ -280,13 +280,13 @@ void lov_fix_desc_pattern(__u32 *val);
 void lov_fix_desc_qos_maxage(__u32 *val);
 __u16 lov_get_stripe_count(struct lov_obd *lov, __u32 magic,
                           __u16 stripe_count);
-int lov_connect_obd(struct obd_device *obd, __u32 index, int activate,
-                    struct obd_connect_data *data);
+int lov_connect_obd(struct obd_device *obd, u32 index, int activate,
+                   struct obd_connect_data *data);
 int lov_setup(struct obd_device *obd, struct lustre_cfg *lcfg);
 int lov_process_config_base(struct obd_device *obd, struct lustre_cfg *lcfg,
-                            __u32 *indexp, int *genp);
-int lov_del_target(struct obd_device *obd, __u32 index,
-                   struct obd_uuid *uuidp, int gen);
+                           u32 *indexp, int *genp);
+int lov_del_target(struct obd_device *obd, u32 index,
+                  struct obd_uuid *uuidp, int gen);
 
 /* lov_pack.c */
 ssize_t lov_lsm_pack(const struct lov_stripe_md *lsm, void *buf,
index c5ac570..51e14f5 100644 (file)
@@ -100,21 +100,21 @@ static void lov_putref(struct obd_device *obd)
 
                list_for_each_entry_safe(tgt, n, &kill, ltd_kill) {
                        list_del(&tgt->ltd_kill);
-                        /* Disconnect */
-                        __lov_del_obd(obd, tgt);
-                }
-        } else {
+                       /* Disconnect */
+                       __lov_del_obd(obd, tgt);
+               }
+       } else {
                mutex_unlock(&lov->lov_lock);
-        }
+       }
 }
 
 static int lov_set_osc_active(struct obd_device *obd, struct obd_uuid *uuid,
-                              enum obd_notify_event ev);
+                             enum obd_notify_event ev);
 static int lov_notify(struct obd_device *obd, struct obd_device *watched,
                      enum obd_notify_event ev);
 
-int lov_connect_obd(struct obd_device *obd, __u32 index, int activate,
-                    struct obd_connect_data *data)
+int lov_connect_obd(struct obd_device *obd, u32 index, int activate,
+                   struct obd_connect_data *data)
 {
        struct lov_obd *lov = &obd->u.lov;
        struct obd_uuid *tgt_uuid;
@@ -316,40 +316,42 @@ static int lov_disconnect_obd(struct obd_device *obd, struct lov_tgt_desc *tgt)
 
 static int lov_disconnect(struct obd_export *exp)
 {
-        struct obd_device *obd = class_exp2obd(exp);
-        struct lov_obd *lov = &obd->u.lov;
-        int i, rc;
-        ENTRY;
-
-        if (!lov->lov_tgts)
-                goto out;
+       struct obd_device *obd = class_exp2obd(exp);
+       struct lov_obd *lov = &obd->u.lov;
+       u32 index;
+       int rc;
 
-        /* Only disconnect the underlying layers on the final disconnect. */
-        lov->lov_connects--;
-        if (lov->lov_connects != 0) {
-                /* why should there be more than 1 connect? */
-                CERROR("disconnect #%d\n", lov->lov_connects);
-                goto out;
-        }
+       ENTRY;
+       if (!lov->lov_tgts)
+               goto out;
+
+       /* Only disconnect the underlying layers on the final disconnect. */
+       lov->lov_connects--;
+       if (lov->lov_connects != 0) {
+               /* why should there be more than 1 connect? */
+               CWARN("%s: unexpected disconnect #%d\n",
+                     obd->obd_name, lov->lov_connects);
+               goto out;
+       }
 
-        /* Let's hold another reference so lov_del_obd doesn't spin through
-           putref every time */
-        obd_getref(obd);
+       /* hold another ref so lov_del_obd() doesn't spin in putref each time */
+       obd_getref(obd);
 
-        for (i = 0; i < lov->desc.ld_tgt_count; i++) {
-                if (lov->lov_tgts[i] && lov->lov_tgts[i]->ltd_exp) {
-                        /* Disconnection is the last we know about an obd */
-                       lov_del_target(obd, i, NULL, lov->lov_tgts[i]->ltd_gen);
-                }
-        }
-        obd_putref(obd);
+       for (index = 0; index < lov->desc.ld_tgt_count; index++) {
+               if (lov->lov_tgts[index] && lov->lov_tgts[index]->ltd_exp) {
+                       /* Disconnection is the last we know about an OBD */
+                       lov_del_target(obd, index, NULL,
+                                      lov->lov_tgts[index]->ltd_gen);
+               }
+       }
+       obd_putref(obd);
 
        if (lov->targets_proc_entry != NULL)
                lprocfs_remove(&lov->targets_proc_entry);
 
 out:
-        rc = class_disconnect(exp); /* bz 9811 */
-        RETURN(rc);
+       rc = class_disconnect(exp); /* bz 9811 */
+       RETURN(rc);
 }
 
 /* Error codes:
@@ -487,37 +489,37 @@ out_notify_lock:
 }
 
 static int lov_add_target(struct obd_device *obd, struct obd_uuid *uuidp,
-                          __u32 index, int gen, int active)
+                         u32 index, int gen, int active)
 {
-        struct lov_obd *lov = &obd->u.lov;
-        struct lov_tgt_desc *tgt;
-        struct obd_device *tgt_obd;
-        int rc;
-        ENTRY;
+       struct lov_obd *lov = &obd->u.lov;
+       struct lov_tgt_desc *tgt;
+       struct obd_device *tgt_obd;
+       int rc;
 
-        CDEBUG(D_CONFIG, "uuid:%s idx:%d gen:%d active:%d\n",
-               uuidp->uuid, index, gen, active);
+       ENTRY;
+       CDEBUG(D_CONFIG, "uuid:%s idx:%u gen:%d active:%d\n",
+              uuidp->uuid, index, gen, active);
 
-        if (gen <= 0) {
-                CERROR("request to add OBD %s with invalid generation: %d\n",
-                       uuidp->uuid, gen);
-                RETURN(-EINVAL);
-        }
+       if (gen <= 0) {
+               CERROR("%s: request to add '%s' with invalid generation: %d\n",
+                      obd->obd_name, uuidp->uuid, gen);
+               RETURN(-EINVAL);
+       }
 
-        tgt_obd = class_find_client_obd(uuidp, LUSTRE_OSC_NAME,
-                                        &obd->obd_uuid);
-        if (tgt_obd == NULL)
-                RETURN(-EINVAL);
+       tgt_obd = class_find_client_obd(uuidp, LUSTRE_OSC_NAME, &obd->obd_uuid);
+       if (tgt_obd == NULL)
+               RETURN(-EINVAL);
 
        mutex_lock(&lov->lov_lock);
 
-        if ((index < lov->lov_tgt_size) && (lov->lov_tgts[index] != NULL)) {
-                tgt = lov->lov_tgts[index];
-                CERROR("UUID %s already assigned at LOV target index %d\n",
-                       obd_uuid2str(&tgt->ltd_uuid), index);
+       if ((index < lov->lov_tgt_size) && (lov->lov_tgts[index] != NULL)) {
+               tgt = lov->lov_tgts[index];
+               rc = -EEXIST;
+               CERROR("%s: UUID %s already assigned at index %d: rc = %d\n",
+                      obd->obd_name, obd_uuid2str(&tgt->ltd_uuid), index, rc);
                mutex_unlock(&lov->lov_lock);
-                RETURN(-EEXIST);
-        }
+               RETURN(rc);
+       }
 
         if (index >= lov->lov_tgt_size) {
                 /* We need to reallocate the lov target array. */
@@ -608,17 +610,17 @@ static int lov_add_target(struct obd_device *obd, struct obd_uuid *uuidp,
                        active ? OBD_NOTIFY_CONNECT : OBD_NOTIFY_INACTIVE);
 
 out:
-        if (rc) {
-                CERROR("add failed (%d), deleting %s\n", rc,
-                       obd_uuid2str(&tgt->ltd_uuid));
+       if (rc) {
+               CERROR("%s: add failed, deleting %s: rc = %d\n",
+                      obd->obd_name, obd_uuid2str(&tgt->ltd_uuid), rc);
                lov_del_target(obd, index, NULL, 0);
-        }
-        obd_putref(obd);
-        RETURN(rc);
+       }
+       obd_putref(obd);
+       RETURN(rc);
 }
 
 /* Schedule a target for deletion */
-int lov_del_target(struct obd_device *obd, __u32 index,
+int lov_del_target(struct obd_device *obd, u32 index,
                    struct obd_uuid *uuidp, int gen)
 {
         struct lov_obd *lov = &obd->u.lov;
@@ -892,43 +894,47 @@ static int lov_cleanup(struct obd_device *obd)
 }
 
 int lov_process_config_base(struct obd_device *obd, struct lustre_cfg *lcfg,
-                            __u32 *indexp, int *genp)
+                           u32 *indexp, int *genp)
 {
-        struct obd_uuid obd_uuid;
-        int cmd;
-        int rc = 0;
-        ENTRY;
+       struct obd_uuid obd_uuid;
+       int cmd;
+       int rc = 0;
 
-        switch(cmd = lcfg->lcfg_command) {
+       ENTRY;
+       switch (cmd = lcfg->lcfg_command) {
        case LCFG_ADD_MDC:
        case LCFG_DEL_MDC:
                break;
        case LCFG_LOV_ADD_OBD:
-        case LCFG_LOV_ADD_INA:
-        case LCFG_LOV_DEL_OBD: {
-                __u32 index;
-                int gen;
-                /* lov_modify_tgts add  0:lov_mdsA  1:ost1_UUID  2:0  3:1 */
-                if (LUSTRE_CFG_BUFLEN(lcfg, 1) > sizeof(obd_uuid.uuid))
-                        GOTO(out, rc = -EINVAL);
-
-                obd_str2uuid(&obd_uuid,  lustre_cfg_buf(lcfg, 1));
-
-               if (sscanf(lustre_cfg_buf(lcfg, 2), "%u", indexp) != 1)
-                        GOTO(out, rc = -EINVAL);
-                if (sscanf(lustre_cfg_buf(lcfg, 3), "%d", genp) != 1)
-                        GOTO(out, rc = -EINVAL);
-                index = *indexp;
-                gen = *genp;
-                if (cmd == LCFG_LOV_ADD_OBD)
-                        rc = lov_add_target(obd, &obd_uuid, index, gen, 1);
-                else if (cmd == LCFG_LOV_ADD_INA)
-                        rc = lov_add_target(obd, &obd_uuid, index, gen, 0);
-                else
-                        rc = lov_del_target(obd, index, &obd_uuid, gen);
-                GOTO(out, rc);
-        }
-        case LCFG_PARAM: {
+       case LCFG_LOV_ADD_INA:
+       case LCFG_LOV_DEL_OBD: {
+               u32 index;
+               int gen;
+
+               /* lov_modify_tgts add  0:lov_mdsA  1:ost1_UUID  2:0  3:1 */
+               if (LUSTRE_CFG_BUFLEN(lcfg, 1) > sizeof(obd_uuid.uuid))
+                       GOTO(out, rc = -EINVAL);
+
+               obd_str2uuid(&obd_uuid,  lustre_cfg_buf(lcfg, 1));
+
+               rc = kstrtou32(lustre_cfg_buf(lcfg, 2), 10, indexp);
+               if (rc)
+                       GOTO(out, rc);
+               rc = kstrtoint(lustre_cfg_buf(lcfg, 3), 10, genp);
+               if (rc)
+                       GOTO(out, rc);
+               index = *indexp;
+               gen = *genp;
+               if (cmd == LCFG_LOV_ADD_OBD)
+                       rc = lov_add_target(obd, &obd_uuid, index, gen, 1);
+               else if (cmd == LCFG_LOV_ADD_INA)
+                       rc = lov_add_target(obd, &obd_uuid, index, gen, 0);
+               else
+                       rc = lov_del_target(obd, index, &obd_uuid, gen);
+
+               GOTO(out, rc);
+       }
+       case LCFG_PARAM: {
                struct lov_desc *desc = &(obd->u.lov.desc);
 
                if (!desc)
index 0ebc4ef..3672ed1 100644 (file)
@@ -702,7 +702,7 @@ int mdc_enqueue_send(const struct lu_env *env, struct obd_export *exp,
                        RETURN(ELDLM_OK);
 
                matched = ldlm_handle2lock(&lockh);
-               if (ldlm_is_kms_ignore(matched))
+               if (!matched || ldlm_is_kms_ignore(matched))
                        goto no_match;
 
                if (mdc_set_dom_lock_data(env, matched, einfo->ei_cbdata)) {