Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752613AbdLLVoN (ORCPT ); Tue, 12 Dec 2017 16:44:13 -0500 Received: from mout.web.de ([212.227.15.3]:61997 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752547AbdLLVoL (ORCPT ); Tue, 12 Dec 2017 16:44:11 -0500 Subject: [PATCH 1/8] target/iscsi: Less function calls in chap_server_compute_md5() after error detection From: SF Markus Elfring To: target-devel@vger.kernel.org, linux-scsi@vger.kernel.org, Al Viro , Arun Easi , Bart Van Assche , Dan Carpenter , David Disseldorp , Hannes Reinecke , Ingo Molnar , "Jason A. Donenfeld" , Jiang Yi , Kees Cook , "Nicholas A. Bellinger" , Russell King , Tang Wenji , "Theodore Ts'o" , Varun Prakash Cc: LKML , kernel-janitors@vger.kernel.org References: <6163538d-a406-2f60-11a2-88b4694e9975@users.sourceforge.net> Message-ID: <5a981970-0474-52a4-5415-bedda030f17a@users.sourceforge.net> Date: Tue, 12 Dec 2017 22:42:49 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <6163538d-a406-2f60-11a2-88b4694e9975@users.sourceforge.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 8bit X-Provags-ID: V03:K0:q/VEWKH6y31a4EFb3a0uKl7PSI8ukL7sowdyWuWMixVZEXDkyez TXIWk0fT445RLAekSa/by4+iJl3q4hASV7XXb71FrJnL4cBzoMT99G9cASF7vVL2jd+JPgw gXXrZ4+dutvxYnEU7LMHf2NjUU7CAhuuygsSd5v5Qg7YHILPFxmM7pdLA7wgVM0NofYB+FK ky+94fwU1eTGO/uRugx8g== X-UI-Out-Filterresults: notjunk:1;V01:K0:wRpeTeMEOwo=:ffx3s1LLE6PMJOzoxl5RkE GimGsCb6DnENLRadIci64SwvU7TlVKW3pPOMPEWNAV94T6AICsig4af5T00eIANvPwAGLnS5h A7/LXrb8FUVvmGxJbX5MnrncIvnkgqwcJ8agB2XmpgoYcEGEgfC8sXDdBRPplO3mhWiNhNgxK 0J0b/xCq204vL4gc/Bpehbc1UiDSED0qaW5sSNF1Jpkv8rQkI/5DULzgDcf3HTOvKUDNYjq1I G/fo4ONf+r0UyBErQRo0uIzZ2BpELWNdz08mzbM+apOcsunM3pxck8t3QyLyLknx6cFRdt3rB cdfxWoLJjYcbQMbDVgKW2Khh1nuWCqZyN62wU6FOnYxgvdOrYmZ85fyQAt8qR083jgrROTnmC WgleNbuF4wzJ9nkr87YUwfrDgyK7TpXafRiotoGaMv13eCWdoRRPcQ3o5WxqEjuXd5zF5aisc qU5ZXT7nqZyvt6Oik9+3GaO2fizPBSyBebaLvqy8wzYO2tmDR/gRjliTuhwDPKwZPOcEqrCte OZFaNGyfXOi3DV/Ezlc1OIQ9/tMY/cxrfRhNZ0UCzlBPCyP2sRAT4kagIKBGL7FUDxjVXoKtw xY7iNoy+8styo4YxISgFJGcTy37uFcH2bd6b5plqq/FvjXC/LZLdaKmaPIrKQc240jst4Czq0 9KqHYgPIo6XTAQhIlyBHImQF3MFNfP6I/+E5ksaGTrj2QhiTejyFotH4IlEEUygY1QKLw7sjf qkd1whAHr8bzJcTIWPEypnUW9HB9OzGUN2g+XxkTlHZUPr8ZbLTp4tUiAe6xzfIfRUjd/cILm pgYjtrCilo9PiHfG1QdoTZ7qSqmyRwL9YozpGSpy8MFS7Ugtnk= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8103 Lines: 280 From: Markus Elfring Date: Tue, 12 Dec 2017 18:00:41 +0100 The functions "crypto_free_shash", "kfree" and "kzfree" were called in a few cases by the chap_server_compute_md5() function during error handling even if the passed variable contained a null pointer. * Adjust jump targets according to the Linux coding style convention. * Delete initialisations for the variables "challenge", "challenge_binhex", "desc" and "tfm" at the beginning which became unnecessary with this refactoring. Fixes: 69110e3cedbb8aad1c70d91ed58a9f4f0ed9eec6 ("iscsi-target: Use shash and ahash") Fixes: e48354ce078c079996f89d715dfa44814b4eba01 ("iscsi-target: Add iSCSI fabric support for target v4.1") Signed-off-by: Markus Elfring --- drivers/target/iscsi/iscsi_target_auth.c | 71 +++++++++++++++++--------------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/drivers/target/iscsi/iscsi_target_auth.c b/drivers/target/iscsi/iscsi_target_auth.c index f9bc8ec6fb6b..94b011fe74e8 100644 --- a/drivers/target/iscsi/iscsi_target_auth.c +++ b/drivers/target/iscsi/iscsi_target_auth.c @@ -186,15 +186,15 @@ static int chap_server_compute_md5( unsigned char id_as_uchar; unsigned char digest[MD5_SIGNATURE_SIZE]; unsigned char type, response[MD5_SIGNATURE_SIZE * 2 + 2]; - unsigned char identifier[10], *challenge = NULL; - unsigned char *challenge_binhex = NULL; + unsigned char identifier[10], *challenge; + unsigned char *challenge_binhex; unsigned char client_digest[MD5_SIGNATURE_SIZE]; unsigned char server_digest[MD5_SIGNATURE_SIZE]; unsigned char chap_n[MAX_CHAP_N_SIZE], chap_r[MAX_RESPONSE_LENGTH]; size_t compare_len; struct iscsi_chap *chap = conn->auth_protocol; - struct crypto_shash *tfm = NULL; - struct shash_desc *desc = NULL; + struct crypto_shash *tfm; + struct shash_desc *desc; int auth_ret = -1, ret, challenge_len; memset(identifier, 0, 10); @@ -208,13 +208,13 @@ static int chap_server_compute_md5( challenge = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL); if (!challenge) { pr_err("Unable to allocate challenge buffer\n"); - goto out; + goto exit; } challenge_binhex = kzalloc(CHAP_CHALLENGE_STR_LEN, GFP_KERNEL); if (!challenge_binhex) { pr_err("Unable to allocate challenge_binhex buffer\n"); - goto out; + goto free_challenge; } /* * Extract CHAP_N. @@ -222,18 +222,18 @@ static int chap_server_compute_md5( if (extract_param(nr_in_ptr, "CHAP_N", MAX_CHAP_N_SIZE, chap_n, &type) < 0) { pr_err("Could not find CHAP_N.\n"); - goto out; + goto free_challenge_binhex; } if (type == HEX) { pr_err("Could not find CHAP_N.\n"); - goto out; + goto free_challenge_binhex; } /* Include the terminating NULL in the compare */ compare_len = strlen(auth->userid) + 1; if (strncmp(chap_n, auth->userid, compare_len) != 0) { pr_err("CHAP_N values do not match!\n"); - goto out; + goto free_challenge_binhex; } pr_debug("[server] Got CHAP_N=%s\n", chap_n); /* @@ -242,11 +242,11 @@ static int chap_server_compute_md5( if (extract_param(nr_in_ptr, "CHAP_R", MAX_RESPONSE_LENGTH, chap_r, &type) < 0) { pr_err("Could not find CHAP_R.\n"); - goto out; + goto free_challenge_binhex; } if (type != HEX) { pr_err("Could not find CHAP_R.\n"); - goto out; + goto free_challenge_binhex; } pr_debug("[server] Got CHAP_R=%s\n", chap_r); @@ -254,15 +254,14 @@ static int chap_server_compute_md5( tfm = crypto_alloc_shash("md5", 0, 0); if (IS_ERR(tfm)) { - tfm = NULL; pr_err("Unable to allocate struct crypto_shash\n"); - goto out; + goto free_challenge_binhex; } desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(tfm), GFP_KERNEL); if (!desc) { pr_err("Unable to allocate struct shash_desc\n"); - goto out; + goto free_shash; } desc->tfm = tfm; @@ -271,27 +270,27 @@ static int chap_server_compute_md5( ret = crypto_shash_init(desc); if (ret < 0) { pr_err("crypto_shash_init() failed\n"); - goto out; + goto free_desc; } ret = crypto_shash_update(desc, &chap->id, 1); if (ret < 0) { pr_err("crypto_shash_update() failed for id\n"); - goto out; + goto free_desc; } ret = crypto_shash_update(desc, (char *)&auth->password, strlen(auth->password)); if (ret < 0) { pr_err("crypto_shash_update() failed for password\n"); - goto out; + goto free_desc; } ret = crypto_shash_finup(desc, chap->challenge, CHAP_CHALLENGE_LENGTH, server_digest); if (ret < 0) { pr_err("crypto_shash_finup() failed for challenge\n"); - goto out; + goto free_desc; } chap_binaryhex_to_asciihex(response, server_digest, MD5_SIGNATURE_SIZE); @@ -299,7 +298,7 @@ static int chap_server_compute_md5( if (memcmp(server_digest, client_digest, MD5_SIGNATURE_SIZE) != 0) { pr_debug("[server] MD5 Digests do not match!\n\n"); - goto out; + goto free_desc; } else pr_debug("[server] MD5 Digests match, CHAP connection" " successful.\n\n"); @@ -309,14 +308,14 @@ static int chap_server_compute_md5( */ if (!auth->authenticate_target) { auth_ret = 0; - goto out; + goto free_desc; } /* * Get CHAP_I. */ if (extract_param(nr_in_ptr, "CHAP_I", 10, identifier, &type) < 0) { pr_err("Could not find CHAP_I.\n"); - goto out; + goto free_desc; } if (type == HEX) @@ -326,11 +325,11 @@ static int chap_server_compute_md5( if (ret < 0) { pr_err("kstrtoul() failed for CHAP identifier: %d\n", ret); - goto out; + goto free_desc; } if (id > 255) { pr_err("chap identifier: %lu greater than 255\n", id); - goto out; + goto free_desc; } /* * RFC 1994 says Identifier is no more than octet (8 bits). @@ -342,23 +341,23 @@ static int chap_server_compute_md5( if (extract_param(nr_in_ptr, "CHAP_C", CHAP_CHALLENGE_STR_LEN, challenge, &type) < 0) { pr_err("Could not find CHAP_C.\n"); - goto out; + goto free_desc; } if (type != HEX) { pr_err("Could not find CHAP_C.\n"); - goto out; + goto free_desc; } pr_debug("[server] Got CHAP_C=%s\n", challenge); challenge_len = chap_string_to_hex(challenge_binhex, challenge, strlen(challenge)); if (!challenge_len) { pr_err("Unable to convert incoming challenge\n"); - goto out; + goto free_desc; } if (challenge_len > 1024) { pr_err("CHAP_C exceeds maximum binary size of 1024 bytes\n"); - goto out; + goto free_desc; } /* * During mutual authentication, the CHAP_C generated by the @@ -368,7 +367,7 @@ static int chap_server_compute_md5( if (!memcmp(challenge_binhex, chap->challenge, CHAP_CHALLENGE_LENGTH)) { pr_err("initiator CHAP_C matches target CHAP_C, failing" " login attempt\n"); - goto out; + goto free_desc; } /* * Generate CHAP_N and CHAP_R for mutual authentication. @@ -376,7 +375,7 @@ static int chap_server_compute_md5( ret = crypto_shash_init(desc); if (ret < 0) { pr_err("crypto_shash_init() failed\n"); - goto out; + goto free_desc; } /* To handle both endiannesses */ @@ -384,7 +383,7 @@ static int chap_server_compute_md5( ret = crypto_shash_update(desc, &id_as_uchar, 1); if (ret < 0) { pr_err("crypto_shash_update() failed for id\n"); - goto out; + goto free_desc; } ret = crypto_shash_update(desc, auth->password_mutual, @@ -392,7 +391,7 @@ static int chap_server_compute_md5( if (ret < 0) { pr_err("crypto_shash_update() failed for" " password_mutual\n"); - goto out; + goto free_desc; } /* * Convert received challenge to binary hex. @@ -401,7 +400,7 @@ static int chap_server_compute_md5( digest); if (ret < 0) { pr_err("crypto_shash_finup() failed for ma challenge\n"); - goto out; + goto free_desc; } /* @@ -419,11 +418,15 @@ static int chap_server_compute_md5( *nr_out_len += 1; pr_debug("[server] Sending CHAP_R=0x%s\n", response); auth_ret = 0; -out: +free_desc: kzfree(desc); +free_shash: crypto_free_shash(tfm); - kfree(challenge); +free_challenge_binhex: kfree(challenge_binhex); +free_challenge: + kfree(challenge); +exit: return auth_ret; } -- 2.15.1