From patchwork Mon Jun 29 18:31:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mauricio Faria de Oliveira X-Patchwork-Id: 1318976 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49wbfz36dpz9sDX; Tue, 30 Jun 2020 04:31:39 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1jpyZ5-00079L-KK; Mon, 29 Jun 2020 18:31:35 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1jpyYz-00076s-PA for kernel-team@lists.ubuntu.com; Mon, 29 Jun 2020 18:31:29 +0000 Received: from mail-qv1-f70.google.com ([209.85.219.70]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jpyYz-0008Nk-E1 for kernel-team@lists.ubuntu.com; Mon, 29 Jun 2020 18:31:29 +0000 Received: by mail-qv1-f70.google.com with SMTP id j6so12344396qvl.13 for ; Mon, 29 Jun 2020 11:31:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=YZ2HArP3kTjhqLFzkFs5eUrhB1GiKVarZKatAasAqp8=; b=Tj7CSddb5FzSHHVQfO5MDby1tnIQAvwQ9B7kgfmOjq6XpifrwdcW/7SrjmEWnoe9WM T+qvo/1ppsAp0XXpr40+xMbIqCW/xEPAZhTUsYLzOeo/Cgzjm7wv4xrNHza54101zlml iUxb55BiXAZjvklp4H5ZjmpGnrMOdk/FzfRRm6zBtK1VyUWg9BWBol4cqRg0LjflPgev 7GVVTpaiJDD9yuRyT1jhvlwD+MuaOD5MBZd1712skfnKt9Sx2DMRjMSW0Dm1mcGx/mMH JA/84tAtZw05r9Sc/zRbJfuJMYY/hf8gNqFQBZiKtRYmfBkWrfQ8/pnDr6oVmiw53Ke9 rVBg== X-Gm-Message-State: AOAM530z5MrwRqwsLAs8IbYv92AiaeFMe5+On4v+pcxq1g0WLbTLzdta mMGSXpZECgZYD0WHCIAohAGqJZsGpTMVIL29Tfg/u9OdfDJd9BMPnNUDfRm8NDFiuXBStLhlJbJ AHBCn4uNlHjH2srtnvBkzrJwJrkc4Fs19EWorFIW5XA== X-Received: by 2002:a37:dd5:: with SMTP id 204mr16368344qkn.438.1593455488277; Mon, 29 Jun 2020 11:31:28 -0700 (PDT) X-Google-Smtp-Source: ABdhPJx5IbDUNnHkZFktcl/HMFbHgGGbTqfJApj2WxYPyk67WXaiqCt7m53KwZ4CElQTRo32JrsjIA== X-Received: by 2002:a37:dd5:: with SMTP id 204mr16368315qkn.438.1593455487961; Mon, 29 Jun 2020 11:31:27 -0700 (PDT) Received: from localhost.localdomain ([201.82.49.101]) by smtp.gmail.com with ESMTPSA id y40sm563376qtc.29.2020.06.29.11.31.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 29 Jun 2020 11:31:27 -0700 (PDT) From: Mauricio Faria de Oliveira To: kernel-team@lists.ubuntu.com Subject: [F/G/Unstable][PATCH 1/1] aufs: do not call i_readcount_inc() Date: Mon, 29 Jun 2020 15:31:22 -0300 Message-Id: <20200629183122.1533457-2-mfo@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200629183122.1533457-1-mfo@canonical.com> References: <20200629183122.1533457-1-mfo@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" The 'struct inode.i_readcount' field is maintained at the VFS, and should not be modified by filesystems. But aufs does in one place, which causes it to be unbalanced. This started with Linux v2.6.39 commit 890275b5eb79 ("IMA: maintain i_readcount in the VFS layer"), which moved the i_readcount updates from IMA into the VFS (at the same places IMA was called previously) and introduced 'mutex_lock(i_mutex)' in the ima_file_check() path. The former change is functionally equivalent, thus no changes are needed in response to it. The latter change, on the other hand, is _not_; and is reported to cause a deadlock in aufs (see below), thus it dropped the call to ima_file_check(). However, when dropping the ima_file_check() call, aufs introduced the i_readcount_inc() call as well, which according to the commit changes is not necessary. This can be observed in aufs2-standalone.git commit 1dbd1c864e455 ("aufs2.1 standalone version for linux-2.6."), announced to the aufs-users mailing list on 2011-04-04 [1]. diff --git a/ChangeLog b/ChangeLog ... +commit 17eac367b03334e57a93e8051eb712add24d2534 +Author: J. R. Okajima +Date: Fri Apr 1 16:31:22 2011 +0900 + + aufs: for 2.6.39, limit the support for IMA + + Since it acquires i_mutex and causes a deadlock, replace a + ima_file_check() call by i_readcount_inc(). + + Signed-off-by: J. R. Okajima ... diff --git a/fs/aufs/vfsub.c b/fs/aufs/vfsub.c ... struct file *vfsub_dentry_open(struct path *path, int flags) ... + if (!IS_ERR_OR_NULL(file) + && (file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) + i_readcount_inc(path->dentry->d_inode); - err = ima_file_check(file, au_conv_oflags(flags)); ... Apparently, this might have been a misunderstanding of one hunk in the 2.6.39 commit, that deletes the lines to increment i_readcount, and adds the lines to acquire i_mutex. It reuses code from the removed function ima_counts_get() to create ima_rdwr_violation_check(), and another hunk calls the new function from ima_file_check(). But note that the i_readcount increment was _not_ called from ima_file_check() previously, via ima_counts_get(): -void ima_counts_get(struct file *file) +static void ima_rdwr_violation_check(struct file *file) { ... + mutex_lock(&inode->i_mutex); /* file metadata: permissions, xattr */ ... - atomic_inc(&inode->i_readcount); #@@ -318,6 +308,7 @@ int ima_file_check(struct file *file, int mask) ... + ima_rdwr_violation_check(file); So, in order to avoid the unbalance caused to i_readcount, drop the i_readcount_inc() call. Note the issue is not the lack of a corresponding i_readcount_dec() call; it's the mere usage of these functions outside of VFS layer, where i_readcount is maintained. Links: [1] https://sourceforge.net/p/aufs/mailman/message/27304125/ snippet: """ aufs2 Monday GIT release From: - 2011-04-04 04:59:18 o news - begin supporting linux-2.6.39-rcN. ... - aufs2-2.6.git#aufs2.1 branch ... aufs: for 2.6.39, limit the support for IMA ... """ Signed-off-by: Mauricio Faria de Oliveira (cherry picked from commit 515a586eeef31e0717d5dea21e2c11a965340b3c aufs4-linux.git) CVE-2020-11935 Signed-off-by: Mauricio Faria de Oliveira --- fs/aufs/vfsub.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/fs/aufs/vfsub.c b/fs/aufs/vfsub.c index e954cd7d0110..a5e10c5c004f 100644 --- a/fs/aufs/vfsub.c +++ b/fs/aufs/vfsub.c @@ -76,15 +76,8 @@ int vfsub_update_h_iattr(struct path *h_path, int *did) struct file *vfsub_dentry_open(struct path *path, int flags) { - struct file *file; - - file = dentry_open(path, flags /* | __FMODE_NONOTIFY */, + return dentry_open(path, flags /* | __FMODE_NONOTIFY */, current_cred()); - if (!IS_ERR_OR_NULL(file) - && (file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) - i_readcount_inc(d_inode(path->dentry)); - - return file; } struct file *vfsub_filp_open(const char *path, int oflags, int mode)