From: Sebastien Buisson Date: Tue, 21 May 2013 09:07:02 +0000 (+0200) Subject: LU-2217 build: fix 'NULL pointer dereference' errors X-Git-Tag: 2.4.93~58 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=a926e593b0c769d20aef191e2b0b6b1d881b6637 LU-2217 build: fix 'NULL pointer dereference' errors 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) Signed-off-by: Sebastien Buisson Change-Id: I646d75d488b66ed348ec51cc2b69f0b644474c30 Reviewed-on: http://review.whamcloud.com/4720 Reviewed-by: Dmitry Eremin Tested-by: Hudson Tested-by: Maloo Reviewed-by: Oleg Drokin --- diff --git a/lnet/klnds/o2iblnd/o2iblnd.c b/lnet/klnds/o2iblnd/o2iblnd.c index 667e8f2..3f5ff4e 100644 --- a/lnet/klnds/o2iblnd/o2iblnd.c +++ b/lnet/klnds/o2iblnd/o2iblnd.c @@ -3224,7 +3224,7 @@ kiblnd_startup (lnet_ni_t *ni) return 0; failed: - if (net->ibn_dev == NULL && ibdev != NULL) + if (net != NULL && net->ibn_dev == NULL && ibdev != NULL) kiblnd_destroy_dev(ibdev); kiblnd_shutdown(ni); diff --git a/lnet/lnet/lib-move.c b/lnet/lnet/lib-move.c index 0906690..b8c817e 100644 --- a/lnet/lnet/lib-move.c +++ b/lnet/lnet/lib-move.c @@ -163,6 +163,7 @@ lnet_iov_nob (unsigned int niov, struct iovec *iov) { unsigned int nob = 0; + LASSERT(niov == 0 || iov != NULL); while (niov-- > 0) nob += (iov++)->iov_len; @@ -323,6 +324,7 @@ lnet_kiov_nob (unsigned int niov, lnet_kiov_t *kiov) { unsigned int nob = 0; + LASSERT(niov == 0 || kiov != NULL); while (niov-- > 0) nob += (kiov++)->kiov_len; diff --git a/lnet/selftest/conctl.c b/lnet/selftest/conctl.c index 5d3a3fd..3b7a96e 100644 --- a/lnet/selftest/conctl.c +++ b/lnet/selftest/conctl.c @@ -671,46 +671,47 @@ 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 == NULL || - (args->lstio_sta_namep == NULL && - args->lstio_sta_idsp == NULL) || - args->lstio_sta_nmlen <= 0 || - args->lstio_sta_nmlen > LST_NAME_SIZE) - return -EINVAL; - - if (args->lstio_sta_idsp != NULL && - args->lstio_sta_count <= 0) - return -EINVAL; - - LIBCFS_ALLOC(name, args->lstio_sta_nmlen + 1); - if (name == NULL) - return -ENOMEM; + if (args->lstio_sta_resultp == NULL) + return -EINVAL; - 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 != NULL) { + if (args->lstio_sta_count <= 0) + return -EINVAL; - if (args->lstio_sta_idsp == NULL) { - rc = lstcon_group_stat(name, args->lstio_sta_timeout, - args->lstio_sta_resultp); - } else { - rc = lstcon_nodes_stat(args->lstio_sta_count, + 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 != NULL) { + 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 == NULL) + return -ENOMEM; - return rc; + rc = copy_from_user(name, args->lstio_sta_namep, + args->lstio_sta_nmlen); + if (rc == 0) + rc = lstcon_group_stat(name, args->lstio_sta_timeout, + args->lstio_sta_resultp); + else + rc = -EFAULT; + + } else { + rc = -EINVAL; + } + + if (name != NULL) + LIBCFS_FREE(name, args->lstio_sta_nmlen + 1); + return rc; } int lst_test_add_ioctl(lstio_test_args_t *args) diff --git a/lustre/fld/lproc_fld.c b/lustre/fld/lproc_fld.c index 67dbb77..80710f6 100644 --- a/lustre/fld/lproc_fld.c +++ b/lustre/fld/lproc_fld.c @@ -344,7 +344,6 @@ static int fldb_seq_release(struct inode *inode, struct file *file) iops = &obj->do_index_ops->dio_it; LASSERT(iops != NULL); - LASSERT(obj != NULL); LASSERT(param->fsp_it != NULL); iops->fini(¶m->fsp_env, param->fsp_it); lu_env_fini(¶m->fsp_env); diff --git a/lustre/include/lustre/lustre_user.h b/lustre/include/lustre/lustre_user.h index 32fc998..5c9e0a7 100644 --- a/lustre/include/lustre/lustre_user.h +++ b/lustre/include/lustre/lustre_user.h @@ -462,6 +462,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 == NULL) + 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/lustre/ldlm/ldlm_request.c b/lustre/ldlm/ldlm_request.c index 49af914..ce51910 100644 --- a/lustre/ldlm/ldlm_request.c +++ b/lustre/ldlm/ldlm_request.c @@ -451,8 +451,13 @@ int ldlm_cli_enqueue_local(struct ldlm_namespace *ns, lock->l_policy_data = *policy; if (client_cookie != NULL) lock->l_client_cookie = *client_cookie; - if (type == LDLM_EXTENT) - lock->l_req_extent = policy->l_extent; + if (type == LDLM_EXTENT) { + /* extent lock without policy is a bug */ + if (policy == NULL) + LBUG(); + + lock->l_req_extent = policy->l_extent; + } err = ldlm_lock_enqueue(ns, &lock, policy, flags); if (unlikely(err != ELDLM_OK)) @@ -910,8 +915,13 @@ int ldlm_cli_enqueue(struct obd_export *exp, struct ptlrpc_request **reqp, lock->l_policy_data = *policy; } - if (einfo->ei_type == LDLM_EXTENT) - lock->l_req_extent = policy->l_extent; + if (einfo->ei_type == LDLM_EXTENT) { + /* extent lock without policy is a bug */ + if (policy == NULL) + LBUG(); + + lock->l_req_extent = policy->l_extent; + } LDLM_DEBUG(lock, "client-side enqueue START, flags %llx\n", *flags); } diff --git a/lustre/lmv/lmv_obd.c b/lustre/lmv/lmv_obd.c index 3877c76..7f8166e 100644 --- a/lustre/lmv/lmv_obd.c +++ b/lustre/lmv/lmv_obd.c @@ -253,7 +253,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 != NULL && (data->ocd_connect_flags & OBD_CONNECT_REAL)) rc = lmv_check_connect(obd); #ifdef __KERNEL__ diff --git a/lustre/lod/lod_object.c b/lustre/lod/lod_object.c index ea9cde7..9c12c5b 100644 --- a/lustre/lod/lod_object.c +++ b/lustre/lod/lod_object.c @@ -483,8 +483,7 @@ static int lod_xattr_set_lov_on_dir(const struct lu_env *env, l->ldo_def_stripe_size = 0; l->ldo_def_stripenr = 0; - LASSERT(buf); - LASSERT(buf->lb_buf); + LASSERT(buf != NULL && buf->lb_buf != NULL); lum = buf->lb_buf; rc = lod_verify_striping(d, buf, 0); diff --git a/lustre/lov/lov_request.c b/lustre/lov/lov_request.c index 608307e..43b52b1 100644 --- a/lustre/lov/lov_request.c +++ b/lustre/lov/lov_request.c @@ -192,7 +192,7 @@ 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 != NULL && tgt->ltd_active) + if (tgt->ltd_active) return 1; return 0; diff --git a/lustre/mdc/mdc_lib.c b/lustre/mdc/mdc_lib.c index 28c598f..ca85704 100644 --- a/lustre/mdc/mdc_lib.c +++ b/lustre/mdc/mdc_lib.c @@ -229,41 +229,41 @@ void mdc_open_pack(struct ptlrpc_request *req, struct md_op_data *op_data, rec = req_capsule_client_get(&req->rq_pill, &RMF_REC_REINT); /* XXX do something about time, uid, gid */ - rec->cr_opcode = REINT_OPEN; - rec->cr_fsuid = current_fsuid(); - rec->cr_fsgid = current_fsgid(); - rec->cr_cap = cfs_curproc_cap_pack(); + rec->cr_opcode = REINT_OPEN; + rec->cr_fsuid = current_fsuid(); + rec->cr_fsgid = current_fsgid(); + rec->cr_cap = cfs_curproc_cap_pack(); + rec->cr_mode = mode; + cr_flags = mds_pack_open_flags(flags, mode); + rec->cr_rdev = rdev; + rec->cr_umask = current_umask(); if (op_data != NULL) { - rec->cr_fid1 = op_data->op_fid1; + rec->cr_fid1 = op_data->op_fid1; + rec->cr_fid2 = op_data->op_fid2; + rec->cr_time = op_data->op_mod_time; + rec->cr_suppgid1 = op_data->op_suppgids[0]; + rec->cr_suppgid2 = op_data->op_suppgids[1]; + rec->cr_bias = op_data->op_bias; + rec->cr_old_handle = op_data->op_handle; + + mdc_pack_capa(req, &RMF_CAPA1, op_data->op_capa1); + /* the next buffer is child capa, which is used for replay, + * will be packed from the data in reply message. */ + + if (op_data->op_name) { + tmp = req_capsule_client_get(&req->rq_pill, &RMF_NAME); + LOGL0(op_data->op_name, op_data->op_namelen, tmp); + if (op_data->op_bias & MDS_CREATE_VOLATILE) + cr_flags |= MDS_OPEN_VOLATILE; + } +#ifndef __KERNEL__ + /*XXX a hack for liblustre to set EA (LL_IOC_LOV_SETSTRIPE) */ rec->cr_fid2 = op_data->op_fid2; - } - rec->cr_mode = mode; - cr_flags = mds_pack_open_flags(flags, mode); - rec->cr_rdev = rdev; - rec->cr_time = op_data->op_mod_time; - rec->cr_suppgid1 = op_data->op_suppgids[0]; - rec->cr_suppgid2 = op_data->op_suppgids[1]; - rec->cr_bias = op_data->op_bias; - rec->cr_umask = current_umask(); - rec->cr_old_handle = op_data->op_handle; - - mdc_pack_capa(req, &RMF_CAPA1, op_data->op_capa1); - /* the next buffer is child capa, which is used for replay, - * will be packed from the data in reply message. */ - - if (op_data->op_name) { - tmp = req_capsule_client_get(&req->rq_pill, &RMF_NAME); - LOGL0(op_data->op_name, op_data->op_namelen, tmp); - if (op_data->op_bias & MDS_CREATE_VOLATILE) - cr_flags |= MDS_OPEN_VOLATILE; +#endif } if (lmm) { cr_flags |= MDS_OPEN_HAS_EA; -#ifndef __KERNEL__ - /*XXX a hack for liblustre to set EA (LL_IOC_LOV_SETSTRIPE) */ - rec->cr_fid2 = op_data->op_fid2; -#endif tmp = req_capsule_client_get(&req->rq_pill, &RMF_EADATA); memcpy(tmp, lmm, lmmlen); } diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index 9108cfb..0fc4a34 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -428,7 +428,7 @@ void mdt_pack_attr2body(struct mdt_thread_info *info, struct mdt_body *b, if (info) mdt_body_reverse_idmap(info, b); - if (b->valid & OBD_MD_FLSIZE) + if (fid != NULL && (b->valid & OBD_MD_FLSIZE)) CDEBUG(D_VFSTRACE, DFID": returning size %llu\n", PFID(fid), (unsigned long long)b->size); } diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c index 5c42b38..03a8d64 100644 --- a/lustre/mdt/mdt_open.c +++ b/lustre/mdt/mdt_open.c @@ -457,7 +457,7 @@ static int mdt_ioepoch_close(struct mdt_thread_info *info, struct mdt_object *o) RETURN(mdt_ioepoch_close_on_eviction(info, o)); if (lustre_msg_get_flags(req->rq_reqmsg) & MSG_REPLAY) RETURN(mdt_ioepoch_close_on_replay(info, o)); - if (info->mti_ioepoch->flags & MF_EPOCH_CLOSE) + if (info->mti_ioepoch && (info->mti_ioepoch->flags & MF_EPOCH_CLOSE)) RETURN(mdt_ioepoch_close_reg(info, o)); /* IO epoch is not closed. */ RETURN(MDT_IOEPOCH_OPENED); @@ -503,7 +503,8 @@ int mdt_som_au_close(struct mdt_thread_info *info, struct mdt_object *o) ioepoch = info->mti_ioepoch ? info->mti_ioepoch->ioepoch : o->mot_ioepoch; - if (!(lustre_msg_get_flags(req->rq_reqmsg) & MSG_REPLAY)) + if (req != NULL + && !(lustre_msg_get_flags(req->rq_reqmsg) & MSG_REPLAY)) rc = mdt_som_attr_set(info, o, ioepoch, act); mdt_object_som_enable(o, ioepoch); } diff --git a/lustre/mgc/mgc_request.c b/lustre/mgc/mgc_request.c index c7d39bc..f2e3026 100644 --- a/lustre/mgc/mgc_request.c +++ b/lustre/mgc/mgc_request.c @@ -360,7 +360,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)) GOTO(out_err3, rc = PTR_ERR(recover_cld)); diff --git a/lustre/obdclass/lprocfs_status.c b/lustre/obdclass/lprocfs_status.c index 62e2714..46eb347 100644 --- a/lustre/obdclass/lprocfs_status.c +++ b/lustre/obdclass/lprocfs_status.c @@ -2221,18 +2221,20 @@ int lprocfs_write_frac_u64_helper(const char *buffer, unsigned long count, } units = 1; - switch(*end) { - case 'p': case 'P': - units <<= 10; - case 't': case 'T': - units <<= 10; - case 'g': case 'G': - units <<= 10; - case 'm': case 'M': - units <<= 10; - case 'k': case 'K': - units <<= 10; - } + if (end != NULL) { + switch (*end) { + case 'p': case 'P': + units <<= 10; + case 't': case 'T': + units <<= 10; + case 'g': case 'G': + units <<= 10; + case 'm': case 'M': + units <<= 10; + case 'k': case 'K': + units <<= 10; + } + } /* Specified units override the multiplier */ if (units) mult = mult < 0 ? -units : units; diff --git a/lustre/ofd/lproc_ofd.c b/lustre/ofd/lproc_ofd.c index 089271a..e11a81b 100644 --- a/lustre/ofd/lproc_ofd.c +++ b/lustre/ofd/lproc_ofd.c @@ -148,9 +148,10 @@ static int lprocfs_ofd_rd_precreate_batch(char *page, char **start, off_t off, int count, int *eof, void *data) { struct obd_device *obd = (struct obd_device *)data; - struct ofd_device *ofd = ofd_dev(obd->obd_lu_dev); + struct ofd_device *ofd; LASSERT(obd != NULL); + ofd = ofd_dev(obd->obd_lu_dev); *eof = 1; return snprintf(page, count, "%d\n", ofd->ofd_precreate_batch); } diff --git a/lustre/ofd/ofd_obd.c b/lustre/ofd/ofd_obd.c index f4f8bcd..fd1f6e0 100644 --- a/lustre/ofd/ofd_obd.c +++ b/lustre/ofd/ofd_obd.c @@ -316,7 +316,7 @@ static int ofd_obd_connect(const struct lu_env *env, struct obd_export **_exp, } CDEBUG(D_HA, "%s: get connection from MDS %d\n", obd->obd_name, - data->ocd_group); + data ? data->ocd_group : -1); out: if (rc != 0) { diff --git a/lustre/osd-ldiskfs/osd_internal.h b/lustre/osd-ldiskfs/osd_internal.h index 7022073..3d47521 100644 --- a/lustre/osd-ldiskfs/osd_internal.h +++ b/lustre/osd-ldiskfs/osd_internal.h @@ -607,6 +607,9 @@ extern int ldiskfs_pdo; static inline int __osd_xattr_get(struct inode *inode, struct dentry *dentry, const char *name, void *buf, int len) { + if (inode == NULL) + return -EINVAL; + dentry->d_inode = inode; dentry->d_sb = inode->i_sb; return inode->i_op->getxattr(dentry, name, buf, len); diff --git a/lustre/osd-ldiskfs/osd_scrub.c b/lustre/osd-ldiskfs/osd_scrub.c index a74ca9b..ca40fe2 100644 --- a/lustre/osd-ldiskfs/osd_scrub.c +++ b/lustre/osd-ldiskfs/osd_scrub.c @@ -1074,7 +1074,7 @@ next: scrub->os_pos_current = param->gbase + ++(param->offset); wait: - if (it != NULL && it->ooi_waiting && + if (it != NULL && it->ooi_waiting && ooc != NULL && ooc->ooc_pos_preload < scrub->os_pos_current) { spin_lock(&scrub->os_lock); it->ooi_waiting = 0; diff --git a/lustre/osp/lproc_osp.c b/lustre/osp/lproc_osp.c index dad0b8d..22e3372 100644 --- a/lustre/osp/lproc_osp.c +++ b/lustre/osp/lproc_osp.c @@ -402,6 +402,9 @@ static int osp_rd_destroys_in_flight(char *page, char **start, off_t off, struct obd_device *dev = data; struct osp_device *osp = lu2osp_dev(dev->obd_lu_dev); + if (osp == NULL) + return -EINVAL; + /* * This counter used to determine if OST has space returned. * Now we need to wait for the following: diff --git a/lustre/osp/osp_sync.c b/lustre/osp/osp_sync.c index 97e5042..9591742 100644 --- a/lustre/osp/osp_sync.c +++ b/lustre/osp/osp_sync.c @@ -997,7 +997,8 @@ static void osp_sync_llog_fini(const struct lu_env *env, struct osp_device *d) struct llog_ctxt *ctxt; ctxt = llog_get_context(d->opd_obd, LLOG_MDS_OST_ORIG_CTXT); - llog_cat_close(env, ctxt->loc_handle); + if (ctxt != NULL) + llog_cat_close(env, ctxt->loc_handle); llog_cleanup(env, ctxt); } diff --git a/lustre/ptlrpc/layout.c b/lustre/ptlrpc/layout.c index d0c4863..0c5f148 100644 --- a/lustre/ptlrpc/layout.c +++ b/lustre/ptlrpc/layout.c @@ -1896,7 +1896,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); diff --git a/lustre/ptlrpc/sec_config.c b/lustre/ptlrpc/sec_config.c index 4685289..773a115 100644 --- a/lustre/ptlrpc/sec_config.c +++ b/lustre/ptlrpc/sec_config.c @@ -751,11 +751,13 @@ void sptlrpc_conf_log_update_begin(const char *logname) mutex_lock(&sptlrpc_conf_lock); conf = sptlrpc_conf_get(fsname, 0); - if (conf && conf->sc_local) { - LASSERT(conf->sc_updated == 0); - sptlrpc_conf_free_rsets(conf); - } - conf->sc_modified = 0; + if (conf) { + if (conf->sc_local) { + LASSERT(conf->sc_updated == 0); + sptlrpc_conf_free_rsets(conf); + } + conf->sc_modified = 0; + } mutex_unlock(&sptlrpc_conf_lock); } diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c index 4439c8a..ed8ce97 100644 --- a/lustre/utils/liblustreapi.c +++ b/lustre/utils/liblustreapi.c @@ -438,7 +438,8 @@ static int get_param_obdvar(const char *fsname, const char *file_path, continue; strcpy(dev, tmp); tmp = strchr(dev, ' '); - *tmp = '\0'; + if (tmp != NULL) + *tmp = '\0'; break; } } @@ -557,10 +558,14 @@ int llapi_search_ost(char *fsname, char *poolname, char *ostname) if (ostname != NULL) len = strlen(ostname); - if (poolname == NULL) - rc = find_target_obdpath(fsname, buffer); - else + if (poolname == NULL) { + if (len == 0) + rc = -EINVAL; + else + rc = find_target_obdpath(fsname, buffer); + } else { rc = find_poolpath(fsname, poolname, buffer); + } if (rc) return rc; diff --git a/lustre/utils/nidlist.c b/lustre/utils/nidlist.c index d9690a3..78e70ce 100644 --- a/lustre/utils/nidlist.c +++ b/lustre/utils/nidlist.c @@ -208,7 +208,9 @@ static char *nl_nid_lookup_ipaddr(char *nid) NI_NAMEREQD | NI_NOFQDN) == 0) { if ((p = strchr(name, '.'))) *p = '\0'; - len = strlen(name) + strlen(lnet) + 2; + len = strlen(name) + 2; + if (lnet) + len += strlen(lnet); if (!(res = malloc(len))) nl_oom(); snprintf(res, len, "%s@%s", name, lnet); diff --git a/lustre/utils/obd.c b/lustre/utils/obd.c index 3b35a77..4860d5d 100644 --- a/lustre/utils/obd.c +++ b/lustre/utils/obd.c @@ -2859,13 +2859,16 @@ static int jt_blockdev_find_module(const char *module) FILE *fp; int found = 0; char buf[1024]; + char *ptr; fp = fopen("/proc/modules", "r"); if (fp == NULL) return -1; while (fgets(buf, 1024, fp) != NULL) { - *strchr(buf, ' ') = 0; + ptr = strchr(buf, ' '); + if (ptr != NULL) + *ptr = 0; if (strcmp(module, buf) == 0) { found = 1; break;