From patchwork Sat Jun 8 13:57:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 10983195 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id CBB5315E6 for ; Sat, 8 Jun 2019 13:57:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B7D99204FE for ; Sat, 8 Jun 2019 13:57:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AC2FB28414; Sat, 8 Jun 2019 13:57: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=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=unavailable 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 4DB0C204FE for ; Sat, 8 Jun 2019 13:57:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727065AbfFHN52 (ORCPT ); Sat, 8 Jun 2019 09:57:28 -0400 Received: from mail-wr1-f65.google.com ([209.85.221.65]:33521 "EHLO mail-wr1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726870AbfFHN51 (ORCPT ); Sat, 8 Jun 2019 09:57:27 -0400 Received: by mail-wr1-f65.google.com with SMTP id n9so4871686wru.0; Sat, 08 Jun 2019 06:57:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=/1vPqIkIXh0cBgOxsLF0x0C4UPurMgv+EOxDVlTh8LY=; b=ZGxJ2LgblftFOTd/1zR8WGgpVqhcNTpaByaTLj6mZbEH/UcT0bUmNtUefWXgNCiLno j8qDwPrir/85hEx82o+C5Z5x717raSkIcyPk/AwwSaZt/GkIyOhNFobCeq34WojemyMo cYzdxwh1i2JraGnN/7Zp2h9GQ6vvTRaBMwKIpQm0htZQqElCzEmslHhT6+Uxs+sWhCYq mIauT7cmrJN2A65SWAMtUdEIRswIoy3Ua3UIUyc0xMbYUStc5wYHv7w7mOIIGBe5JhA5 G1WfHM2URX/t4BvbLFRl/UEtCiMEO3P81Vtvx7Z5f4UEHDLNmgXb9j6OQHj1Tr7DHCqF MLaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=/1vPqIkIXh0cBgOxsLF0x0C4UPurMgv+EOxDVlTh8LY=; b=dZmx0UF2rOxjM7WaAojC0zysenDYfRPmSLDqjE13KXx8TdG8Uaq6negsQ1N3SzlUgg ytcxCSBAMB6kY33T+HL+vpcrX/4GSaFNfNiWj7UfD95J1MtKFL5TouvKo/zS4/RbeLGq LLI8UhafmUKt4JvZUtbsZ08qNk/pg85wqKBpWIDpELaW9nCaDwDLIrg9aXNyg7G194cU vXdZsec44mi1n0zsCy/33oa6HeHtTVsg9EIqBzodBdAX7Z4tZTNUbuhpxgR+ashE8C8R EKgImdb7SEidNNVLXbSuNbmR/I4mDTno0XmzfQW8Km1/A0pBsJ9fAwsKJR2aVzBhTAyA WDQQ== X-Gm-Message-State: APjAAAVC+7TuHkzPtnNWs+xOBMqfmZa/1tA1GtLYNYmH+EOrV+N2d3Yq dAruc4Nd+tyOzl76UY3ME3QBnZnH X-Google-Smtp-Source: APXvYqzmCedMUjGCbciuyHJaRtFqGtqi/WiTE1xhJs7a0rVzbX1Bw+LMkdMf/o9KzUAu4QmpnGPy6A== X-Received: by 2002:adf:ee48:: with SMTP id w8mr15486046wro.308.1560002245557; Sat, 08 Jun 2019 06:57:25 -0700 (PDT) Received: from localhost.localdomain ([5.102.238.208]) by smtp.gmail.com with ESMTPSA id j132sm9423463wmj.21.2019.06.08.06.57.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 08 Jun 2019 06:57:25 -0700 (PDT) From: Amir Goldstein To: Miklos Szeredi Cc: "J . Bruce Fields" , Jeff Layton , Mimi Zohar , Al Viro , linux-fsdevel@vger.kernel.org, linux-unionfs@vger.kernel.org, linux-integrity@vger.kernel.org Subject: [PATCH 1/2] vfs: replace i_readcount with a biased i_count Date: Sat, 8 Jun 2019 16:57:16 +0300 Message-Id: <20190608135717.8472-2-amir73il@gmail.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190608135717.8472-1-amir73il@gmail.com> References: <20190608135717.8472-1-amir73il@gmail.com> Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Count struct files open RO together with inode reference count instead of using a dedicated i_readcount field. This will allow us to use the RO count also when CONFIG_IMA is not defined and will reduce the size of struct inode for 32bit archs when CONFIG_IMA is defined. We need this RO count for posix leases code, which currently naively checks i_count and d_count in an inaccurate manner. Should regular i_count overflow into RO count bias by struct files opened for write, it's not a big deal, as we mostly need the RO count to be reliable when the first writer comes along. Cc: # v4.19 Signed-off-by: Amir Goldstein --- include/linux/fs.h | 33 +++++++++++++++++++------------ security/integrity/ima/ima_main.c | 2 +- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index f7fdfe93e25d..504bf17967dd 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -694,9 +694,6 @@ struct inode { atomic_t i_count; atomic_t i_dio_count; atomic_t i_writecount; -#ifdef CONFIG_IMA - atomic_t i_readcount; /* struct files open RO */ -#endif union { const struct file_operations *i_fop; /* former ->i_op->default_file_ops */ void (*free_inode)(struct inode *); @@ -2890,26 +2887,36 @@ static inline bool inode_is_open_for_write(const struct inode *inode) return atomic_read(&inode->i_writecount) > 0; } -#ifdef CONFIG_IMA +/* + * Count struct files open RO together with inode rerefernce count. + * We need this count for IMA and for posix leases. The RO count should not + * include files opened RDWR nor files opened O_PATH and internal kernel + * inode references, like the ones taken by overlayfs and inotify. + * Should regular i_count overflow into I_RO_COUNT_BIAS by struct files + * opened for write, it's not a big deal, as we mostly need + * inode_is_open_rdonly() to be reliable when the first writer comes along. + */ +#define I_RO_COUNT_SHIFT 10 +#define I_RO_COUNT_BIAS (1UL << I_RO_COUNT_SHIFT) + static inline void i_readcount_dec(struct inode *inode) { - BUG_ON(!atomic_read(&inode->i_readcount)); - atomic_dec(&inode->i_readcount); + WARN_ON(atomic_read(&inode->i_count) < I_RO_COUNT_BIAS); + atomic_sub(I_RO_COUNT_BIAS, &inode->i_count); } static inline void i_readcount_inc(struct inode *inode) { - atomic_inc(&inode->i_readcount); + atomic_add(I_RO_COUNT_BIAS, &inode->i_count); } -#else -static inline void i_readcount_dec(struct inode *inode) +static inline int i_readcount_read(const struct inode *inode) { - return; + return atomic_read(&inode->i_count) >> I_RO_COUNT_SHIFT; } -static inline void i_readcount_inc(struct inode *inode) +static inline bool inode_is_open_rdonly(const struct inode *inode) { - return; + return atomic_read(&inode->i_count) > I_RO_COUNT_BIAS; } -#endif + extern int do_pipe_flags(int *, int); #define __kernel_read_file_id(id) \ diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 357edd140c09..766bac778d11 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -94,7 +94,7 @@ static void ima_rdwr_violation_check(struct file *file, bool send_tomtou = false, send_writers = false; if (mode & FMODE_WRITE) { - if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) { + if (inode_is_open_rdonly(inode) && IS_IMA(inode)) { if (!iint) iint = integrity_iint_find(inode); /* IMA_MEASURE is set from reader side */ From patchwork Sat Jun 8 13:57:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 10983199 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2433215E6 for ; Sat, 8 Jun 2019 13:57:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1256E204FE for ; Sat, 8 Jun 2019 13:57:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0652A28414; Sat, 8 Jun 2019 13:57:31 +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=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=unavailable 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 99B1228161 for ; Sat, 8 Jun 2019 13:57:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726870AbfFHN53 (ORCPT ); Sat, 8 Jun 2019 09:57:29 -0400 Received: from mail-wr1-f68.google.com ([209.85.221.68]:38844 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726227AbfFHN53 (ORCPT ); Sat, 8 Jun 2019 09:57:29 -0400 Received: by mail-wr1-f68.google.com with SMTP id d18so4835361wrs.5; Sat, 08 Jun 2019 06:57:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=Ugquv+cQpSiI2Qu8XC8CXWcqfkSghoxUuZbFae3uhPE=; b=tpseAOirJpzZcaIdP0O3lk8ADXE5nTAhJAYEEypOy3rwNBAJifmiaa1lkBKFiB8Epd gXGbWnWJFJNliTOfkKeGKJyAFYM8giDZ0o8E/mG2d2xC5zr7VYteyQ2BVUvNCdTolGdE GCnwZiqoNGonQiDrr81fHNo2UnccHE6+ZQ/5B3RoLbNEkehICdEu1+5sa0SU8qqQimSq mODl/sdWTNMIkg1aA03wgUzqP3Pmq4LfD4H1JnpI3KS0CwsPzLr9ys6t74ginRiXf0QE lPZ4cA3U4Y9N07GZ36nOwZ52MF4WmMl0LPYRsmL2Rf6tD1ZKJtAWiBbykQbgxmOuSSKk cfLg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=Ugquv+cQpSiI2Qu8XC8CXWcqfkSghoxUuZbFae3uhPE=; b=Ty3b395k2jpG7my50NjO9rHN3zB/HItidbYl0Z6lMuu7j3fHwmocIJOV+uE5fagHm8 PG80N5FQAKVDNSvoQCFnWac5qmNcpkcdwF0ax7sUh4DU6szIvuxGOKvBVZwYb7/1MyJJ 7f9VHsFXDwgXZltaNRvd0kyXk4FUtcnS5D6kS4YMyXpDcsnRTf5waGBqocN/7Ms+x1Vg AusKUyOH2H7AMofyxPZbpB+GNwNWx+lQRokcg+mjzvPEdKxNzZ1UVhj73zyv2WPYaKhZ zcJgSS/yBQzQWKJMiHLG0lxYGFIM9hlLGwIc5RrlGs/frEUevSkYTIktIgL2V2mnGbYF ilsQ== X-Gm-Message-State: APjAAAUPMOtpB/rIhVvDlexxlPTTmRZE3HCKLAOj/zAlbjdYCf0CGkxf CEWkmr+ldTM0zshpF2ihI+8= X-Google-Smtp-Source: APXvYqzcqnRXXwVJkdzJSKVS39IDGQNSHq4J3r+dmbhJ1cq1hSj+iq9BgjqiZY4ybYxp+L7A5MTkig== X-Received: by 2002:a5d:4e50:: with SMTP id r16mr11232468wrt.197.1560002246945; Sat, 08 Jun 2019 06:57:26 -0700 (PDT) Received: from localhost.localdomain ([5.102.238.208]) by smtp.gmail.com with ESMTPSA id j132sm9423463wmj.21.2019.06.08.06.57.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 08 Jun 2019 06:57:26 -0700 (PDT) From: Amir Goldstein To: Miklos Szeredi Cc: "J . Bruce Fields" , Jeff Layton , Mimi Zohar , Al Viro , linux-fsdevel@vger.kernel.org, linux-unionfs@vger.kernel.org, linux-integrity@vger.kernel.org Subject: [PATCH 2/2] locks: eliminate false positive conflicts for write lease Date: Sat, 8 Jun 2019 16:57:17 +0300 Message-Id: <20190608135717.8472-3-amir73il@gmail.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190608135717.8472-1-amir73il@gmail.com> References: <20190608135717.8472-1-amir73il@gmail.com> Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP check_conflicting_open() is checking for existing fd's open for read or for write before allowing to take a write lease. The check that was implemented using i_count and d_count is an approximation that has several false positives. For example, overlayfs since v4.19, takes an extra reference on the dentry; An open with O_PATH takes a reference on the inode and dentry although the file cannot be read nor written. Change the implementation to use inode_is_open_rdonly() and i_writecount to eliminate the false positive conflicts and allow a write lease to be taken on an overlayfs file. The change of behavior with existing fd's open with O_PATH is symmetric w.r.t. current behavior of lease breakers - an open with O_PATH currently does not break a write lease. Cc: # v4.19 Signed-off-by: Amir Goldstein --- fs/locks.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/fs/locks.c b/fs/locks.c index ec1e4a5df629..4937cfdf611a 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -1753,10 +1753,10 @@ int fcntl_getlease(struct file *filp) } /** - * check_conflicting_open - see if the given dentry points to a file that has + * check_conflicting_open - see if the given file points to an inode that has * an existing open that would conflict with the * desired lease. - * @dentry: dentry to check + * @filp: file to check * @arg: type of lease that we're trying to acquire * @flags: current lock flags * @@ -1764,10 +1764,11 @@ int fcntl_getlease(struct file *filp) * conflict with the lease we're trying to set. */ static int -check_conflicting_open(const struct dentry *dentry, const long arg, int flags) +check_conflicting_open(struct file *filp, const long arg, int flags) { int ret = 0; - struct inode *inode = dentry->d_inode; + struct inode *inode = locks_inode(filp); + int self_wcount = 0, self_rcount = 0; if (flags & FL_LAYOUT) return 0; @@ -1775,8 +1776,14 @@ check_conflicting_open(const struct dentry *dentry, const long arg, int flags) if ((arg == F_RDLCK) && inode_is_open_for_write(inode)) return -EAGAIN; - if ((arg == F_WRLCK) && ((d_count(dentry) > 1) || - (atomic_read(&inode->i_count) > 1))) + /* Make sure that only read/write count is from lease requestor */ + if (filp->f_mode & FMODE_WRITE) + self_wcount = 1; + else if ((filp->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) + self_rcount = 1; + + if ((arg == F_WRLCK) && (i_readcount_read(inode) != self_rcount || + (atomic_read(&inode->i_writecount) != self_wcount))) ret = -EAGAIN; return ret; @@ -1786,8 +1793,7 @@ static int generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **priv) { struct file_lock *fl, *my_fl = NULL, *lease; - struct dentry *dentry = filp->f_path.dentry; - struct inode *inode = dentry->d_inode; + struct inode *inode = locks_inode(filp); struct file_lock_context *ctx; bool is_deleg = (*flp)->fl_flags & FL_DELEG; int error; @@ -1822,7 +1828,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr percpu_down_read(&file_rwsem); spin_lock(&ctx->flc_lock); time_out_leases(inode, &dispose); - error = check_conflicting_open(dentry, arg, lease->fl_flags); + error = check_conflicting_open(filp, arg, lease->fl_flags); if (error) goto out; @@ -1879,7 +1885,7 @@ generic_add_lease(struct file *filp, long arg, struct file_lock **flp, void **pr * precedes these checks. */ smp_mb(); - error = check_conflicting_open(dentry, arg, lease->fl_flags); + error = check_conflicting_open(filp, arg, lease->fl_flags); if (error) { locks_unlink_lock_ctx(lease); goto out;