From: Sebastien Buisson <sbuisson@xxxxxxx> Fix 'NULL pointer dereference' defects found by Coverity version 6.5.3: Dereference after null check (FORWARD_NULL) For instance, Passing null pointer to a function which dereferences it. Dereference before null check (REVERSE_INULL) Null-checking variable suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Dereference null return value (NULL_RETURNS) The following fixes for the LNet layer are broken out of patch http://review.whamcloud.com/4720. Signed-off-by: Sebastien Buisson <sbuisson@xxxxxxx> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-2217 Reviewed-on: http://review.whamcloud.com/4720 Reviewed-by: Dmitry Eremin <dmitry.eremin@xxxxxxxxx> Reviewed-by: Oleg Drokin <oleg.drokin@xxxxxxxxx> --- drivers/staging/lustre/lnet/lnet/lib-move.c | 2 + drivers/staging/lustre/lnet/selftest/conctl.c | 49 ++++++++++---------- .../lustre/lustre/include/lustre/lustre_user.h | 3 + drivers/staging/lustre/lustre/ldlm/ldlm_request.c | 7 ++- drivers/staging/lustre/lustre/lmv/lmv_obd.c | 2 +- drivers/staging/lustre/lustre/lov/lov_request.c | 2 +- drivers/staging/lustre/lustre/mgc/mgc_request.c | 10 ++++- .../lustre/lustre/obdclass/lprocfs_status.c | 24 +++++---- drivers/staging/lustre/lustre/ptlrpc/layout.c | 2 +- 9 files changed, 61 insertions(+), 40 deletions(-) diff --git a/drivers/staging/lustre/lnet/lnet/lib-move.c b/drivers/staging/lustre/lnet/lnet/lib-move.c index a5e90e7..f323b8b 100644 --- a/drivers/staging/lustre/lnet/lnet/lib-move.c +++ b/drivers/staging/lustre/lnet/lnet/lib-move.c @@ -162,6 +162,7 @@ lnet_iov_nob(unsigned int niov, struct kvec *iov) { unsigned int nob = 0; + LASSERT(!niov || iov); while (niov-- > 0) nob += (iov++)->iov_len; @@ -282,6 +283,7 @@ lnet_kiov_nob(unsigned int niov, lnet_kiov_t *kiov) { unsigned int nob = 0; + LASSERT(!niov || kiov); while (niov-- > 0) nob += (kiov++)->kiov_len; diff --git a/drivers/staging/lustre/lnet/selftest/conctl.c b/drivers/staging/lustre/lnet/selftest/conctl.c index 714d14b..62cacb6 100644 --- a/drivers/staging/lustre/lnet/selftest/conctl.c +++ b/drivers/staging/lustre/lnet/selftest/conctl.c @@ -670,44 +670,45 @@ static int lst_stat_query_ioctl(lstio_stat_args_t *args) { int rc; - char *name; + char *name = NULL; /* TODO: not finished */ if (args->lstio_sta_key != console_session.ses_key) return -EACCES; - if (!args->lstio_sta_resultp || - (!args->lstio_sta_namep && !args->lstio_sta_idsp) || - args->lstio_sta_nmlen <= 0 || - args->lstio_sta_nmlen > LST_NAME_SIZE) - return -EINVAL; - - if (args->lstio_sta_idsp && - args->lstio_sta_count <= 0) + if (!args->lstio_sta_resultp) return -EINVAL; - LIBCFS_ALLOC(name, args->lstio_sta_nmlen + 1); - if (!name) - return -ENOMEM; - - if (copy_from_user(name, args->lstio_sta_namep, - args->lstio_sta_nmlen)) { - LIBCFS_FREE(name, args->lstio_sta_nmlen + 1); - return -EFAULT; - } + if (args->lstio_sta_idsp) { + if (args->lstio_sta_count <= 0) + return -EINVAL; - if (!args->lstio_sta_idsp) { - rc = lstcon_group_stat(name, args->lstio_sta_timeout, - args->lstio_sta_resultp); - } else { rc = lstcon_nodes_stat(args->lstio_sta_count, args->lstio_sta_idsp, args->lstio_sta_timeout, args->lstio_sta_resultp); - } + } else if (args->lstio_sta_namep) { + if (args->lstio_sta_nmlen <= 0 || + args->lstio_sta_nmlen > LST_NAME_SIZE) + return -EINVAL; - LIBCFS_FREE(name, args->lstio_sta_nmlen + 1); + LIBCFS_ALLOC(name, args->lstio_sta_nmlen + 1); + if (!name) + return -ENOMEM; + rc = copy_from_user(name, args->lstio_sta_namep, + args->lstio_sta_nmlen); + if (!rc) + rc = lstcon_group_stat(name, args->lstio_sta_timeout, + args->lstio_sta_resultp); + else + rc = -EFAULT; + } else { + rc = -EINVAL; + } + + if (name) + LIBCFS_FREE(name, args->lstio_sta_nmlen + 1); return rc; } diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h index 9f026bd..276906e 100644 --- a/drivers/staging/lustre/lustre/include/lustre/lustre_user.h +++ b/drivers/staging/lustre/lustre/include/lustre/lustre_user.h @@ -448,6 +448,9 @@ static inline void obd_str2uuid(struct obd_uuid *uuid, const char *tmp) /* For printf's only, make sure uuid is terminated */ static inline char *obd_uuid2str(const struct obd_uuid *uuid) { + if (!uuid) + return NULL; + if (uuid->uuid[sizeof(*uuid) - 1] != '\0') { /* Obviously not safe, but for printfs, no real harm done... * we're always null-terminated, even in a race. diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c index 2d501a6..6f0761c 100644 --- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c @@ -708,8 +708,13 @@ int ldlm_cli_enqueue(struct obd_export *exp, struct ptlrpc_request **reqp, if (policy) lock->l_policy_data = *policy; - if (einfo->ei_type == LDLM_EXTENT) + if (einfo->ei_type == LDLM_EXTENT) { + /* extent lock without policy is a bug */ + if (!policy) + LBUG(); + lock->l_req_extent = policy->l_extent; + } LDLM_DEBUG(lock, "client-side enqueue START, flags %llx\n", *flags); } diff --git a/drivers/staging/lustre/lustre/lmv/lmv_obd.c b/drivers/staging/lustre/lustre/lmv/lmv_obd.c index 5c055a0..267f001 100644 --- a/drivers/staging/lustre/lustre/lmv/lmv_obd.c +++ b/drivers/staging/lustre/lustre/lmv/lmv_obd.c @@ -238,7 +238,7 @@ static int lmv_connect(const struct lu_env *env, * and MDC stuff will be called directly, for instance while reading * ../mdc/../kbytesfree procfs file, etc. */ - if (data->ocd_connect_flags & OBD_CONNECT_REAL) + if (data && data->ocd_connect_flags & OBD_CONNECT_REAL) rc = lmv_check_connect(obd); if (rc && lmv->lmv_tgts_kobj) diff --git a/drivers/staging/lustre/lustre/lov/lov_request.c b/drivers/staging/lustre/lustre/lov/lov_request.c index 4f568f0..7178a02 100644 --- a/drivers/staging/lustre/lustre/lov/lov_request.c +++ b/drivers/staging/lustre/lustre/lov/lov_request.c @@ -178,7 +178,7 @@ static int lov_check_and_wait_active(struct lov_obd *lov, int ost_idx) cfs_time_seconds(1), NULL, NULL); rc = l_wait_event(waitq, lov_check_set(lov, ost_idx), &lwi); - if (tgt && tgt->ltd_active) + if (tgt->ltd_active) return 1; return 0; diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c index f5a85bb..bc49633 100644 --- a/drivers/staging/lustre/lustre/mgc/mgc_request.c +++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c @@ -344,7 +344,15 @@ static int config_log_add(struct obd_device *obd, char *logname, LASSERT(lsi->lsi_lmd); if (!(lsi->lsi_lmd->lmd_flags & LMD_FLG_NOIR)) { struct config_llog_data *recover_cld; - *strrchr(seclogname, '-') = 0; + + ptr = strrchr(seclogname, '-'); + if (ptr != NULL) { + *ptr = 0; + } else { + CERROR("sptlrpc log name not correct: %s", seclogname); + config_log_put(cld); + return -EINVAL; + } recover_cld = config_recover_log_add(obd, seclogname, cfg, sb); if (IS_ERR(recover_cld)) { rc = PTR_ERR(recover_cld); diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c index 7c28755..1ea1578 100644 --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c @@ -1359,17 +1359,19 @@ int lprocfs_write_frac_u64_helper(const char __user *buffer, } units = 1; - switch (tolower(*end)) { - case 'p': - units <<= 10; - case 't': - units <<= 10; - case 'g': - units <<= 10; - case 'm': - units <<= 10; - case 'k': - units <<= 10; + if (end != NULL) { + switch (tolower(*end)) { + case 'p': + units <<= 10; + case 't': + units <<= 10; + case 'g': + units <<= 10; + case 'm': + units <<= 10; + case 'k': + units <<= 10; + } } /* Specified units override the multiplier */ if (units > 1) diff --git a/drivers/staging/lustre/lustre/ptlrpc/layout.c b/drivers/staging/lustre/lustre/ptlrpc/layout.c index bdd9053..5b06901 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/layout.c +++ b/drivers/staging/lustre/lustre/ptlrpc/layout.c @@ -1798,7 +1798,7 @@ swabber_dumper_helper(struct req_capsule *pill, return; swabber(value); ptlrpc_buf_set_swabbed(pill->rc_req, inout, offset); - if (dump) { + if (dump && field->rmf_dumper) { CDEBUG(D_RPCTRACE, "Dump of swabbed field %s follows\n", field->rmf_name); field->rmf_dumper(value); -- 1.7.1 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
