From patchwork Thu Jan 17 11:59:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juerg Haefliger X-Patchwork-Id: 1026573 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43gN0T5Xwvz9sMQ; Thu, 17 Jan 2019 22:59:21 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1gk6KJ-0000Ef-0r; Thu, 17 Jan 2019 11:59:15 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.86_2) (envelope-from ) id 1gk6KD-0000DJ-US for kernel-team@lists.ubuntu.com; Thu, 17 Jan 2019 11:59:09 +0000 Received: from mail-ed1-f70.google.com ([209.85.208.70]) by youngberry.canonical.com with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1gk6KD-0005b3-Mp for kernel-team@lists.ubuntu.com; Thu, 17 Jan 2019 11:59:09 +0000 Received: by mail-ed1-f70.google.com with SMTP id e17so3572691edr.7 for ; Thu, 17 Jan 2019 03:59:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=jxEIK34s8QAXh3H2HbtN27+wIneP+dA4KuJ10sIwoMY=; b=aLYbiLks/HhM0/V3VHjfcXH/s3e0vucZ4ptuHPUVnD1/Nt7AxMm72ERgYsITHQ/zXX DNlpMM42Wm2LsHm0DHJSyPJIwpSbn0Cu5wp1M1DxekFfkgOoWvGAKg7FydVyK+8oC0C6 g/jYbCgPWYjljvUvV1nQiI7K7ddzir5CNoeTSYYG8NUI/hJfK3K6cIvNeIoudMu0c4Tt ZqFHgxwkDpaLCTruYDvS46zcbjohFTcRCkcEiqCDPi5omAAs55CfNwBBKOknQ1f6MoxF WaRq2XFNsfpthunZI4AxeYpb+Me+I6GaJqhS7NfKme+kuTOK+8L0GRxx82PTdrASywu6 0qTQ== X-Gm-Message-State: AJcUukfHsaQ0Oa1xSnjjcFuC/evntbH4B2jT01SlynBfTkB2FMGOeFlH of8g59xJhciw4/Zo2udFfMUh6CSxN2ZLBRme78/moqGZXu617lBVNZN4OYFYusxBuivOUCTWxjj rNCNxCQgNFIEw48yb0kHxhXFZY50J37tRf91JbcNK3A== X-Received: by 2002:a50:d551:: with SMTP id f17mr11257814edj.87.1547726349088; Thu, 17 Jan 2019 03:59:09 -0800 (PST) X-Google-Smtp-Source: ALg8bN74RkUNO/rqD7HiNgLwNrVOhrUk98Sflnsoo61lsAhX9ZfZaZM0tCXTfGvjjB7rbYv47C7qxw== X-Received: by 2002:a50:d551:: with SMTP id f17mr11257801edj.87.1547726348811; Thu, 17 Jan 2019 03:59:08 -0800 (PST) Received: from localhost.localdomain ([81.221.192.120]) by smtp.gmail.com with ESMTPSA id n11sm6292478edn.14.2019.01.17.03.59.08 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 17 Jan 2019 03:59:08 -0800 (PST) From: Juerg Haefliger X-Google-Original-From: Juerg Haefliger To: kernel-team@lists.ubuntu.com Subject: [SRU][Bionic][PATCH 1/3] iscsi target: fix session creation failure handling Date: Thu, 17 Jan 2019 12:59:03 +0100 Message-Id: <20190117115905.20587-2-juergh@canonical.com> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20190117115905.20587-1-juergh@canonical.com> References: <20190117115905.20587-1-juergh@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Mike Christie BugLink: https://bugs.launchpad.net/bugs/1812086 The problem is that iscsi_login_zero_tsih_s1 sets conn->sess early in iscsi_login_set_conn_values. If the function fails later like when we alloc the idr it does kfree(sess) and leaves the conn->sess pointer set. iscsi_login_zero_tsih_s1 then returns -Exyz and we then call iscsi_target_login_sess_out and access the freed memory. This patch has iscsi_login_zero_tsih_s1 either completely setup the session or completely tear it down, so later in iscsi_target_login_sess_out we can just check for it being set to the connection. Cc: stable@vger.kernel.org Fixes: 0957627a9960 ("iscsi-target: Fix sess allocation leak in...") Signed-off-by: Mike Christie Acked-by: Martin K. Petersen Signed-off-by: Matthew Wilcox (cherry picked from commit 26abc916a898d34c5ad159315a2f683def3c5555) Signed-off-by: Juerg Haefliger --- drivers/target/iscsi/iscsi_target_login.c | 35 ++++++++++++++--------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c index 64c5a57b92e4..05801c51c3e2 100644 --- a/drivers/target/iscsi/iscsi_target_login.c +++ b/drivers/target/iscsi/iscsi_target_login.c @@ -348,8 +348,7 @@ static int iscsi_login_zero_tsih_s1( pr_err("idr_alloc() for sess_idr failed\n"); iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, ISCSI_LOGIN_STATUS_NO_RESOURCES); - kfree(sess); - return -ENOMEM; + goto free_sess; } sess->creation_time = get_jiffies_64(); @@ -365,20 +364,28 @@ static int iscsi_login_zero_tsih_s1( ISCSI_LOGIN_STATUS_NO_RESOURCES); pr_err("Unable to allocate memory for" " struct iscsi_sess_ops.\n"); - kfree(sess); - return -ENOMEM; + goto remove_idr; } sess->se_sess = transport_init_session(TARGET_PROT_NORMAL); if (IS_ERR(sess->se_sess)) { iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR, ISCSI_LOGIN_STATUS_NO_RESOURCES); - kfree(sess->sess_ops); - kfree(sess); - return -ENOMEM; + goto free_ops; } return 0; + +free_ops: + kfree(sess->sess_ops); +remove_idr: + spin_lock_bh(&sess_idr_lock); + idr_remove(&sess_idr, sess->session_index); + spin_unlock_bh(&sess_idr_lock); +free_sess: + kfree(sess); + conn->sess = NULL; + return -ENOMEM; } static int iscsi_login_zero_tsih_s2( @@ -1161,13 +1168,13 @@ void iscsi_target_login_sess_out(struct iscsi_conn *conn, ISCSI_LOGIN_STATUS_INIT_ERR); if (!zero_tsih || !conn->sess) goto old_sess_out; - if (conn->sess->se_sess) - transport_free_session(conn->sess->se_sess); - if (conn->sess->session_index != 0) { - spin_lock_bh(&sess_idr_lock); - idr_remove(&sess_idr, conn->sess->session_index); - spin_unlock_bh(&sess_idr_lock); - } + + transport_free_session(conn->sess->se_sess); + + spin_lock_bh(&sess_idr_lock); + idr_remove(&sess_idr, conn->sess->session_index); + spin_unlock_bh(&sess_idr_lock); + kfree(conn->sess->sess_ops); kfree(conn->sess); conn->sess = NULL;