From patchwork Thu Feb 27 21:08:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Simmons X-Patchwork-Id: 11409649 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5F1B3138D for ; Thu, 27 Feb 2020 21:18:24 +0000 (UTC) Received: from pdx1-mailman02.dreamhost.com (pdx1-mailman02.dreamhost.com [64.90.62.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 47865246A1 for ; Thu, 27 Feb 2020 21:18:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 47865246A1 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lustre-devel-bounces@lists.lustre.org Received: from pdx1-mailman02.dreamhost.com (localhost [IPv6:::1]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id A5FA121FA4B; Thu, 27 Feb 2020 13:18:23 -0800 (PST) X-Original-To: lustre-devel@lists.lustre.org Delivered-To: lustre-devel-lustre.org@pdx1-mailman02.dreamhost.com Received: from smtp3.ccs.ornl.gov (smtp3.ccs.ornl.gov [160.91.203.39]) by pdx1-mailman02.dreamhost.com (Postfix) with ESMTP id 17EE321FA60 for ; Thu, 27 Feb 2020 13:18:19 -0800 (PST) Received: from star.ccs.ornl.gov (star.ccs.ornl.gov [160.91.202.134]) by smtp3.ccs.ornl.gov (Postfix) with ESMTP id 55C936DA; Thu, 27 Feb 2020 16:18:13 -0500 (EST) Received: by star.ccs.ornl.gov (Postfix, from userid 2004) id 54011468; Thu, 27 Feb 2020 16:18:13 -0500 (EST) From: James Simmons To: Andreas Dilger , Oleg Drokin , NeilBrown Date: Thu, 27 Feb 2020 16:08:02 -0500 Message-Id: <1582838290-17243-15-git-send-email-jsimmons@infradead.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1582838290-17243-1-git-send-email-jsimmons@infradead.org> References: <1582838290-17243-1-git-send-email-jsimmons@infradead.org> Subject: [lustre-devel] [PATCH 014/622] lustre: mdc: fix possible NULL pointer dereference X-BeenThere: lustre-devel@lists.lustre.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "For discussing Lustre software development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lustre Development List MIME-Version: 1.0 Errors-To: lustre-devel-bounces@lists.lustre.org Sender: "lustre-devel" From: Andreas Dilger Fix two static analysis errors. fs/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. fs/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. WC-bug-id: https://jira.whamcloud.com/browse/LU-10264 Lustre-commit: b89206476174 ("LU-10264 mdc: fix possible NULL pointer dereference") Signed-off-by: Andreas Dilger Reviewed-on: https://review.whamcloud.com/31621 Reviewed-by: Dmitry Eremin Reviewed-by: Bob Glossman Reviewed-by: James Simmons Reviewed-by: Oleg Drokin Signed-off-by: James Simmons --- fs/lustre/lov/lov_obd.c | 45 ++++++++++++++++++++++++--------------------- fs/lustre/mdc/mdc_dev.c | 2 +- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/fs/lustre/lov/lov_obd.c b/fs/lustre/lov/lov_obd.c index 1708fa9..26637bc 100644 --- a/fs/lustre/lov/lov_obd.c +++ b/fs/lustre/lov/lov_obd.c @@ -312,7 +312,8 @@ 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; + u32 index; + int rc; if (!lov->lov_tgts) goto out; @@ -321,19 +322,19 @@ static int lov_disconnect(struct obd_export *exp) lov->lov_connects--; if (lov->lov_connects != 0) { /* why should there be more than 1 connect? */ - CERROR("disconnect #%d\n", lov->lov_connects); + 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 - */ + /* hold another ref so lov_del_obd() doesn't spin in putref each time */ lov_tgts_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); + 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); } } @@ -490,13 +491,12 @@ static int lov_add_target(struct obd_device *obd, struct obd_uuid *uuidp, uuidp->uuid, index, gen, active); if (gen <= 0) { - CERROR("request to add OBD %s with invalid generation: %d\n", - uuidp->uuid, gen); + 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); + tgt_obd = class_find_client_obd(uuidp, LUSTRE_OSC_NAME, &obd->obd_uuid); if (!tgt_obd) return -EINVAL; @@ -504,10 +504,11 @@ static int lov_add_target(struct obd_device *obd, struct obd_uuid *uuidp, if ((index < lov->lov_tgt_size) && lov->lov_tgts[index]) { tgt = lov->lov_tgts[index]; - CERROR("UUID %s already assigned at LOV target index %d\n", - obd_uuid2str(&tgt->ltd_uuid), 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) { @@ -602,8 +603,8 @@ static int lov_add_target(struct obd_device *obd, struct obd_uuid *uuidp, out: if (rc) { - CERROR("add failed (%d), deleting %s\n", rc, - obd_uuid2str(&tgt->ltd_uuid)); + 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); } lov_tgts_putref(obd); @@ -860,6 +861,7 @@ int lov_process_config_base(struct obd_device *obd, struct lustre_cfg *lcfg, 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)) { rc = -EINVAL; @@ -868,11 +870,11 @@ int lov_process_config_base(struct obd_device *obd, struct lustre_cfg *lcfg, obd_str2uuid(&obd_uuid, lustre_cfg_buf(lcfg, 1)); - rc = kstrtoint(lustre_cfg_buf(lcfg, 2), 10, indexp); - if (rc < 0) + rc = kstrtou32(lustre_cfg_buf(lcfg, 2), 10, indexp); + if (rc) goto out; rc = kstrtoint(lustre_cfg_buf(lcfg, 3), 10, genp); - if (rc < 0) + if (rc) goto out; index = *indexp; gen = *genp; @@ -882,6 +884,7 @@ int lov_process_config_base(struct obd_device *obd, struct lustre_cfg *lcfg, rc = lov_add_target(obd, &obd_uuid, index, gen, 0); else rc = lov_del_target(obd, index, &obd_uuid, gen); + goto out; } case LCFG_PARAM: { diff --git a/fs/lustre/mdc/mdc_dev.c b/fs/lustre/mdc/mdc_dev.c index ca0822d..80e3120 100644 --- a/fs/lustre/mdc/mdc_dev.c +++ b/fs/lustre/mdc/mdc_dev.c @@ -684,7 +684,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)) {