From patchwork Thu Mar 5 17:47:59 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dan Carpenter X-Patchwork-Id: 5948581 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id A979DBF440 for ; Thu, 5 Mar 2015 17:48:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C40FB20364 for ; Thu, 5 Mar 2015 17:48:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AD19C20361 for ; Thu, 5 Mar 2015 17:48:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754737AbbCERsO (ORCPT ); Thu, 5 Mar 2015 12:48:14 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:31060 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750884AbbCERsN (ORCPT ); Thu, 5 Mar 2015 12:48:13 -0500 Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id t25HmCQR025157 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 5 Mar 2015 17:48:12 GMT Received: from userz7022.oracle.com (userz7022.oracle.com [156.151.31.86]) by aserv0021.oracle.com (8.13.8/8.13.8) with ESMTP id t25HmBSm016906 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 5 Mar 2015 17:48:11 GMT Received: from abhmp0005.oracle.com (abhmp0005.oracle.com [141.146.116.11]) by userz7022.oracle.com (8.14.5+Sun/8.14.4) with ESMTP id t25HmAl1010813; Thu, 5 Mar 2015 17:48:10 GMT Received: from mwanda (/154.0.139.178) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 05 Mar 2015 09:48:09 -0800 Date: Thu, 5 Mar 2015 20:47:59 +0300 From: Dan Carpenter To: Alexander Viro Cc: linux-fsdevel@vger.kernel.org, ocfs2-devel@oss.oracle.com, kernel-janitors@vger.kernel.org Subject: [patch 2/2 v2] posix_acl: make posix_acl_create() safer and cleaner Message-ID: <20150305174759.GB13294@mwanda> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150124193124.GA18322@mwanda> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If posix_acl_create() returns an error code then "*acl" and "*default_acl" can be uninitialized or point to freed memory. This is a dangerous thing to do. For example, it causes a problem in ocfs2_reflink(): fs/ocfs2/refcounttree.c:4327 ocfs2_reflink() error: potentially using uninitialized 'default_acl'. I've re-written this so we set the pointers to NULL at the start. I've added a temporary "clone" variable to hold the value of "*acl" until end. Setting them to NULL means means we don't need the "no_acl" label. We may as well remove the "apply_umask" stuff forward and remove that label as well. Signed-off-by: Dan Carpenter --- v2: Update to latest kernel. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 3a48bb7..a327300 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -547,51 +547,45 @@ posix_acl_create(struct inode *dir, umode_t *mode, struct posix_acl **default_acl, struct posix_acl **acl) { struct posix_acl *p; + struct posix_acl *clone; int ret; + *acl = NULL; + *default_acl = NULL; + if (S_ISLNK(*mode) || !IS_POSIXACL(dir)) - goto no_acl; + return 0; p = get_acl(dir, ACL_TYPE_DEFAULT); - if (IS_ERR(p)) { - if (p == ERR_PTR(-EOPNOTSUPP)) - goto apply_umask; - return PTR_ERR(p); + if (!p || p == ERR_PTR(-EOPNOTSUPP)) { + *mode &= ~current_umask(); + return 0; } + if (IS_ERR(p)) + return PTR_ERR(p); - if (!p) - goto apply_umask; - - *acl = posix_acl_clone(p, GFP_NOFS); - if (!*acl) + clone = posix_acl_clone(p, GFP_NOFS); + if (!clone) goto no_mem; - ret = posix_acl_create_masq(*acl, mode); + ret = posix_acl_create_masq(clone, mode); if (ret < 0) goto no_mem_clone; - if (ret == 0) { - posix_acl_release(*acl); - *acl = NULL; - } + if (ret == 0) + posix_acl_release(clone); + else + *acl = clone; - if (!S_ISDIR(*mode)) { + if (!S_ISDIR(*mode)) posix_acl_release(p); - *default_acl = NULL; - } else { + else *default_acl = p; - } - return 0; -apply_umask: - *mode &= ~current_umask(); -no_acl: - *default_acl = NULL; - *acl = NULL; return 0; no_mem_clone: - posix_acl_release(*acl); + posix_acl_release(clone); no_mem: posix_acl_release(p); return -ENOMEM;