@@ -48,15 +48,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)
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 <hooanon05@yahoo.co.jp> +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 <hooanon05@yahoo.co.jp> ... 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: <sfjro@us...> - 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 <mfo@canonical.com> (cherry picked from commit 515a586eeef31e0717d5dea21e2c11a965340b3c aufs4-linux.git) CVE-2020-11935 Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> --- fs/aufs/vfsub.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)