From patchwork Fri Jun 8 03:21:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ronnie Sahlberg X-Patchwork-Id: 10453737 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 8D40D601D4 for ; Fri, 8 Jun 2018 03:21:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 71690296FF for ; Fri, 8 Jun 2018 03:21:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 61D78297BC; Fri, 8 Jun 2018 03:21:29 +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.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI 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 75126296FF for ; Fri, 8 Jun 2018 03:21:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752385AbeFHDV1 (ORCPT ); Thu, 7 Jun 2018 23:21:27 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:60792 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752329AbeFHDV0 (ORCPT ); Thu, 7 Jun 2018 23:21:26 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 631CBCB9C0; Fri, 8 Jun 2018 03:21:26 +0000 (UTC) Received: from test1135.test.redhat.com (vpn2-54-65.bne.redhat.com [10.64.54.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 818812024CA1; Fri, 8 Jun 2018 03:21:25 +0000 (UTC) From: Ronnie Sahlberg To: linux-cifs Cc: Steve French Subject: [PATCH] cifs: fix a buffer leak in smb2_query_symlink Date: Fri, 8 Jun 2018 13:21:18 +1000 Message-Id: <20180608032118.15490-1-lsahlber@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Fri, 08 Jun 2018 03:21:26 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Fri, 08 Jun 2018 03:21:26 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lsahlber@redhat.com' RCPT:'' 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 This leak was introduced in 91cb74f5142c14dd921ab2d064b7b128054f9fae and caused us to leak one small buffer for every symlink query. Signed-off-by: Ronnie Sahlberg --- fs/cifs/link.c | 6 ++++-- fs/cifs/smb2file.c | 3 ++- fs/cifs/smb2inode.c | 3 ++- fs/cifs/smb2ops.c | 51 ++++++++++++++++++++++++++++----------------------- fs/cifs/smb2pdu.c | 3 ++- fs/cifs/smb2proto.h | 2 +- 6 files changed, 39 insertions(+), 29 deletions(-) diff --git a/fs/cifs/link.c b/fs/cifs/link.c index 889a840172eb..de41f96aba49 100644 --- a/fs/cifs/link.c +++ b/fs/cifs/link.c @@ -421,7 +421,8 @@ smb3_query_mf_symlink(unsigned int xid, struct cifs_tcon *tcon, return -ENOMEM; } - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, pfile_info, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, pfile_info, NULL, + NULL); if (rc) goto qmf_out_open_fail; @@ -478,7 +479,8 @@ smb3_create_mf_symlink(unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, + NULL); if (rc) { kfree(utf16_path); return rc; diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c index 12af5dba742b..788412675723 100644 --- a/fs/cifs/smb2file.c +++ b/fs/cifs/smb2file.c @@ -64,7 +64,8 @@ smb2_open_file(const unsigned int xid, struct cifs_open_parms *oparms, if (oparms->tcon->ses->server->capabilities & SMB2_GLOBAL_CAP_LEASING) memcpy(smb2_oplock + 1, fid->lease_key, SMB2_LEASE_KEY_SIZE); - rc = SMB2_open(xid, oparms, smb2_path, smb2_oplock, smb2_data, NULL); + rc = SMB2_open(xid, oparms, smb2_path, smb2_oplock, smb2_data, NULL, + NULL); if (rc) goto out; diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c index a6e786e39248..d01ad706d7fc 100644 --- a/fs/cifs/smb2inode.c +++ b/fs/cifs/smb2inode.c @@ -71,7 +71,8 @@ smb2_open_op_close(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, + NULL); if (rc) { kfree(utf16_path); return rc; diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index aec03d586b16..b15f5957d645 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -348,7 +348,7 @@ int open_shroot(unsigned int xid, struct cifs_tcon *tcon, struct cifs_fid *pfid) oparams.fid = pfid; oparams.reconnect = false; - rc = SMB2_open(xid, &oparams, &srch_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparams, &srch_path, &oplock, NULL, NULL, NULL); if (rc == 0) { memcpy(tcon->prfid, pfid, sizeof(struct cifs_fid)); tcon->valid_root_fid = true; @@ -375,7 +375,8 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon) oparms.reconnect = false; if (no_cached_open) - rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL, + NULL); else rc = open_shroot(xid, tcon, &fid); @@ -413,7 +414,7 @@ smb2_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon) oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL, NULL); if (rc) return; @@ -449,7 +450,7 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL); if (rc) { kfree(utf16_path); return rc; @@ -598,7 +599,7 @@ smb2_query_eas(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL); kfree(utf16_path); if (rc) { cifs_dbg(FYI, "open failed rc=%d\n", rc); @@ -677,7 +678,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL); kfree(utf16_path); if (rc) { cifs_dbg(FYI, "open failed rc=%d\n", rc); @@ -1261,7 +1262,7 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL); kfree(utf16_path); if (rc) { cifs_dbg(FYI, "open dir failed rc=%d\n", rc); @@ -1361,7 +1362,7 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, &srch_path, &oplock, NULL, NULL, NULL); if (rc) return rc; buf->f_type = SMB2_MAGIC_NUMBER; @@ -1515,7 +1516,8 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, struct cifs_open_parms oparms; struct cifs_fid fid; struct kvec err_iov = {NULL, 0}; - struct smb2_err_rsp *err_buf; + struct smb2_err_rsp *err_buf = NULL; + int resp_buftype; struct smb2_symlink_err_rsp *symlink; unsigned int sub_len; unsigned int sub_offset; @@ -1535,18 +1537,18 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, &err_iov); - + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, &err_iov, + &resp_buftype); if (!rc || !err_iov.iov_base) { - kfree(utf16_path); - return -ENOENT; + rc = -ENOENT; + goto querty_exit; } err_buf = err_iov.iov_base; if (le32_to_cpu(err_buf->ByteCount) < sizeof(struct smb2_symlink_err_rsp) || err_iov.iov_len < SMB2_SYMLINK_STRUCT_SIZE) { - kfree(utf16_path); - return -ENOENT; + rc = -ENOENT; + goto querty_exit; } /* open must fail on symlink - reset rc */ @@ -1558,25 +1560,28 @@ smb2_query_symlink(const unsigned int xid, struct cifs_tcon *tcon, print_offset = le16_to_cpu(symlink->PrintNameOffset); if (err_iov.iov_len < SMB2_SYMLINK_STRUCT_SIZE + sub_offset + sub_len) { - kfree(utf16_path); - return -ENOENT; + rc = -ENOENT; + goto querty_exit; } if (err_iov.iov_len < SMB2_SYMLINK_STRUCT_SIZE + print_offset + print_len) { - kfree(utf16_path); - return -ENOENT; + rc = -ENOENT; + goto querty_exit; } *target_path = cifs_strndup_from_utf16( (char *)symlink->PathBuffer + sub_offset, sub_len, true, cifs_sb->local_nls); if (!(*target_path)) { - kfree(utf16_path); - return -ENOMEM; + rc = -ENOMEM; + goto querty_exit; } convert_delimiter(*target_path, '/'); cifs_dbg(FYI, "%s: target path: %s\n", __func__, *target_path); + + querty_exit: + free_rsp_buf(resp_buftype, err_buf); kfree(utf16_path); return rc; } @@ -1649,7 +1654,7 @@ get_smb2_acl_by_path(struct cifs_sb_info *cifs_sb, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL); kfree(utf16_path); if (!rc) { rc = SMB2_query_acl(xid, tlink_tcon(tlink), fid.persistent_fid, @@ -1712,7 +1717,7 @@ set_smb2_acl(struct cifs_ntsd *pnntsd, __u32 acllen, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL); kfree(utf16_path); if (!rc) { rc = SMB2_set_acl(xid, tlink_tcon(tlink), fid.persistent_fid, diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 92e54b1638be..48e2004c75fb 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1889,7 +1889,7 @@ alloc_path_with_tree_prefix(__le16 **out_path, int *out_size, int *out_len, int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, __u8 *oplock, struct smb2_file_all_info *buf, - struct kvec *err_iov) + struct kvec *err_iov, int *buftype) { struct smb2_create_req *req; struct smb2_create_rsp *rsp; @@ -2052,6 +2052,7 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, cifs_stats_fail_inc(tcon, SMB2_CREATE_HE); if (err_iov && rsp) { *err_iov = rsp_iov; + *buftype = resp_buftype; resp_buftype = CIFS_NO_BUFFER; rsp = NULL; } diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h index 908555b1c6b5..c84020057bd8 100644 --- a/fs/cifs/smb2proto.h +++ b/fs/cifs/smb2proto.h @@ -125,7 +125,7 @@ extern int SMB2_tdis(const unsigned int xid, struct cifs_tcon *tcon); extern int SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, __u8 *oplock, struct smb2_file_all_info *buf, - struct kvec *err_iov); + struct kvec *err_iov, int *resp_buftype); extern int SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, u64 volatile_fid, u32 opcode, bool is_fsctl, char *in_data, u32 indatalen,