From patchwork Thu Jun 21 16:59:10 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 10480237 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8446060230 for ; Thu, 21 Jun 2018 16:59:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6ACF8291E6 for ; Thu, 21 Jun 2018 16:59:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5D848291F1; Thu, 21 Jun 2018 16:59:34 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AB8A4291E6 for ; Thu, 21 Jun 2018 16:59:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933429AbeFUQ7d (ORCPT ); Thu, 21 Jun 2018 12:59:33 -0400 Received: from mail-pg0-f67.google.com ([74.125.83.67]:38052 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933392AbeFUQ7b (ORCPT ); Thu, 21 Jun 2018 12:59:31 -0400 Received: by mail-pg0-f67.google.com with SMTP id c9-v6so1696394pgf.5; Thu, 21 Jun 2018 09:59:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=DIEIO3LyTYJlFnbkxEi4xlz+eewGtPvgYw/pWfGW5as=; b=FYv8mHM2AuQvamMB0iFDgzyg6X0ldTzcWsU1k1vm5CSKQ5isonkBfLsGA7Gtb9/I0K o/y9y54iALiZUDgMVhqGPKreVCSYwIbxrG2xcSDn35Hg2zv0MS+/2yMwK1hLvw30Ym7c 40sUxOozE7XGVpO96sJCHi/ZADigxJCN/Fd38dF9duaHsVb4BSvEafoMxb2wAKqWkEGU 2ePmxyhsLevU9s5jnKBfveVLOr7vB6RFZjkrZP373jFe5JhRYmIxHipIbPtK4K4qw9Qp xHG38Io3CrnJ9KHDmihDFXSRJpujTZSU4lPuLxUWaV1Vdj6qf1x3sNxBozRSwy18GPbm FRVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=DIEIO3LyTYJlFnbkxEi4xlz+eewGtPvgYw/pWfGW5as=; b=qxFPIagWjxXt5akZksG11MgG5SiiyJYenV4+9d9Y317hYYG5oNL7jvRtaQriZVkZun q9YCsh9eyK/PbvAt+C0B19p+54ELbrKiHv40Bgzgt+GY67GesvXXIhDnpkaE5wcp7kCh LVAY+wtk0gMgL3dsLmEuLa4bwgvFjDrTEGlFGMvmKGSnxLIe+J9BKtNCerIMpApIqP1e PjKXKFk4fZ5h7hY54y4wVnCBUhMK6a2eBhnzjPxAiKWuqE6T6xnIKfF0ajgm2F27c0NJ CWTVcKgdkz/d91ehcNdN8LR8L+np6Di3xyl1zFynF1bS8W7ruoxbwWWbLH+Bq4Mv6W0Q 031Q== X-Gm-Message-State: APt69E06IUt3gFeIo2wu/nUwcKQfMJr5+aPwM6+Dsjs2o3lY0rK0e7Dr 7zW5CiAM7u9k1m0GLbxwlke26chVULFfpPQCcXxGwQ== X-Google-Smtp-Source: ADUXVKJViKirUp1bAkWo+3DE6jDxQbLCE1RC1927YSE3j7sMSgqzc0SBDVZrU0ls29YiI/x6pVyDvqhY5xHXJmy4DJY= X-Received: by 2002:a62:48cd:: with SMTP id q74-v6mr27645333pfi.153.1529600371063; Thu, 21 Jun 2018 09:59:31 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a17:90a:80cc:0:0:0:0 with HTTP; Thu, 21 Jun 2018 09:59:10 -0700 (PDT) In-Reply-To: <20180621133523.GA30313@embeddedor.com> References: <20180621133523.GA30313@embeddedor.com> From: Steve French Date: Thu, 21 Jun 2018 11:59:10 -0500 Message-ID: Subject: Re: [PATCH] CIFS: fix structurally dead code in smb311_posix_mkdir To: "Gustavo A. R. Silva" , Dan Carpenter Cc: Aurelien Aptel , Steve French , CIFS , samba-technical , LKML Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Folded this patch and the equivalent one from Dan in with the comments from Aurelien into one patch and repushed to cifs-2.6.git for-next - see attached. On Thu, Jun 21, 2018 at 8:35 AM, Gustavo A. R. Silva wrote: > Code at line: 1950: rc = -EIO; is unreachable. Hence, the function > returns rc with a value of zero and, this is not the actual intention > of this particular piece of code. > > Fix this by placing the goto instruction just after the update to rc. > > Addresses-Coverity-ID: 1470124 ("Structurally dead code") > Fixes: 5539e9b24a38 ("CIFS: fix memory leak and remove dead code") > Signed-off-by: Gustavo A. R. Silva > --- > fs/cifs/smb2pdu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 062a2a5..c9489b1 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -1946,8 +1946,8 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, > if (ses && (ses->server)) > server = ses->server; > else { > - goto err_free_path; > rc = -EIO; > + goto err_free_path; > } > > /* resource #2: request */ > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html From f877386d373753c83d791d82328356cd98812eb8 Mon Sep 17 00:00:00 2001 From: Aurelien Aptel Date: Tue, 19 Jun 2018 15:18:48 -0700 Subject: [PATCH 1/2] CIFS: fix memory leak and remove dead code also fixes error code in smb311_posix_mkdir() (where the error assignment needs to go before the goto) a typo that Dan Carpenter and Paulo pointed out. Signed-off-by: Aurelien Aptel Signed-off-by: Dan Carpenter Reviewed-by: Paulo Alcantara Signed-off-by: Steve French --- fs/cifs/smb2pdu.c | 99 +++++++++++++++++++++++------------------------ 1 file changed, 49 insertions(+), 50 deletions(-) diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 810b85787c91..ea102dca5f33 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1934,27 +1934,31 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, char *pc_buf = NULL; int flags = 0; unsigned int total_len; - __le16 *path = cifs_convert_path_to_utf16(full_path, cifs_sb); - - if (!path) - return -ENOMEM; + __le16 *utf16_path = NULL; cifs_dbg(FYI, "mkdir\n"); + /* resource #1: path allocation */ + utf16_path = cifs_convert_path_to_utf16(full_path, cifs_sb); + if (!utf16_path) + return -ENOMEM; + if (ses && (ses->server)) server = ses->server; - else - return -EIO; + else { + rc = -EIO; + goto err_free_path; + } + /* resource #2: request */ rc = smb2_plain_req_init(SMB2_CREATE, tcon, (void **) &req, &total_len); - if (rc) - return rc; + goto err_free_path; + if (smb3_encryption_required(tcon)) flags |= CIFS_TRANSFORM_REQ; - req->ImpersonationLevel = IL_IMPERSONATION; req->DesiredAccess = cpu_to_le32(FILE_WRITE_ATTRIBUTES); /* File attributes ignored on open (used in create though) */ @@ -1983,50 +1987,44 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, req->sync_hdr.Flags |= SMB2_FLAGS_DFS_OPERATIONS; rc = alloc_path_with_tree_prefix(©_path, ©_size, &name_len, - tcon->treeName, path); - if (rc) { - cifs_small_buf_release(req); - return rc; - } + tcon->treeName, utf16_path); + if (rc) + goto err_free_req; + req->NameLength = cpu_to_le16(name_len * 2); uni_path_len = copy_size; - path = copy_path; + /* free before overwriting resource */ + kfree(utf16_path); + utf16_path = copy_path; } else { - uni_path_len = (2 * UniStrnlen((wchar_t *)path, PATH_MAX)) + 2; + uni_path_len = (2 * UniStrnlen((wchar_t *)utf16_path, PATH_MAX)) + 2; /* MUST set path len (NameLength) to 0 opening root of share */ req->NameLength = cpu_to_le16(uni_path_len - 2); if (uni_path_len % 8 != 0) { copy_size = roundup(uni_path_len, 8); copy_path = kzalloc(copy_size, GFP_KERNEL); if (!copy_path) { - cifs_small_buf_release(req); - return -ENOMEM; + rc = -ENOMEM; + goto err_free_req; } - memcpy((char *)copy_path, (const char *)path, + memcpy((char *)copy_path, (const char *)utf16_path, uni_path_len); uni_path_len = copy_size; - path = copy_path; + /* free before overwriting resource */ + kfree(utf16_path); + utf16_path = copy_path; } } iov[1].iov_len = uni_path_len; - iov[1].iov_base = path; + iov[1].iov_base = utf16_path; req->RequestedOplockLevel = SMB2_OPLOCK_LEVEL_NONE; if (tcon->posix_extensions) { - if (n_iov > 2) { - struct create_context *ccontext = - (struct create_context *)iov[n_iov-1].iov_base; - ccontext->Next = - cpu_to_le32(iov[n_iov-1].iov_len); - } - + /* resource #3: posix buf */ rc = add_posix_context(iov, &n_iov, mode); - if (rc) { - cifs_small_buf_release(req); - kfree(copy_path); - return rc; - } + if (rc) + goto err_free_req; pc_buf = iov[n_iov-1].iov_base; } @@ -2035,32 +2033,33 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, rqst.rq_iov = iov; rqst.rq_nvec = n_iov; - rc = cifs_send_recv(xid, ses, &rqst, &resp_buftype, flags, - &rsp_iov); - - cifs_small_buf_release(req); - rsp = (struct smb2_create_rsp *)rsp_iov.iov_base; - - if (rc != 0) { + /* resource #4: response buffer */ + rc = cifs_send_recv(xid, ses, &rqst, &resp_buftype, flags, &rsp_iov); + if (rc) { cifs_stats_fail_inc(tcon, SMB2_CREATE_HE); trace_smb3_posix_mkdir_err(xid, tcon->tid, ses->Suid, - CREATE_NOT_FILE, FILE_WRITE_ATTRIBUTES, rc); - goto smb311_mkdir_exit; - } else - trace_smb3_posix_mkdir_done(xid, rsp->PersistentFileId, tcon->tid, - ses->Suid, CREATE_NOT_FILE, - FILE_WRITE_ATTRIBUTES); + CREATE_NOT_FILE, + FILE_WRITE_ATTRIBUTES, rc); + goto err_free_rsp_buf; + } + + rsp = (struct smb2_create_rsp *)rsp_iov.iov_base; + trace_smb3_posix_mkdir_done(xid, rsp->PersistentFileId, tcon->tid, + ses->Suid, CREATE_NOT_FILE, + FILE_WRITE_ATTRIBUTES); SMB2_close(xid, tcon, rsp->PersistentFileId, rsp->VolatileFileId); /* Eventually save off posix specific response info and timestaps */ -smb311_mkdir_exit: - kfree(copy_path); - kfree(pc_buf); +err_free_rsp_buf: free_rsp_buf(resp_buftype, rsp); + kfree(pc_buf); +err_free_req: + cifs_small_buf_release(req); +err_free_path: + kfree(utf16_path); return rc; - } #endif /* SMB311 */ -- 2.17.1