Skip to content

GitLab

Switch branch/tag
  1. 18 Jun, 2020 3 commits
    • J. R. Okajima's avatar
      Merge branch 'aufs5.0/00base' into aufs5.0/01modular · c0380002
      J. R. Okajima authored
      c0380002
    • J. R. Okajima's avatar
      aufs: bugfix, IMA i_readcount · d8e4eb08
      J. R. Okajima authored 
      

By the recent commit
	21913077f9918 2020-06-17 aufs: do not call i_readcount_inc()
a very old bug was fixed, which is inblance counter.
But still aufs needs to call i_readcount_inc() when the branch
permission is chaned from RW to RO.  Otherwise the counter reaches 0
and BUG() in i_readcount_dec() will be activated.
Signed-off-by: default avatarJ. R. Okajima <hooanon05g@gmail.com>
(cherry picked from commit f10aea57d39d6cd311312e9e7746804f7059b5c8)
      d8e4eb08
    • Mauricio Faria de Oliveira's avatar
      aufs: do not call i_readcount_inc() · 72a59459
      Mauricio Faria de Oliveira authored 
      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: default avatarMauricio Faria de Oliveira <mfo@canonical.com>
(cherry picked from commit 515a586eeef31e0717d5dea21e2c11a965340b3c)
      72a59459
  3. 13 May, 2020 2 commits
  5. 20 Apr, 2020 2 commits
  7. 29 Feb, 2020 2 commits
  9. 22 Jan, 2020 2 commits
  11. 16 Jan, 2020 3 commits
  13. 31 Dec, 2019 2 commits
  15. 18 Dec, 2019 2 commits
  17. 15 Dec, 2019 2 commits
  19. 02 Dec, 2019 2 commits
  21. 08 Oct, 2019 2 commits
    • J. R. Okajima's avatar
      Merge branch 'aufs5.0/00base' into aufs5.0/01modular · be53027f
      J. R. Okajima authored
      be53027f
    • J. R. Okajima's avatar
      aufs: possible bugfix, uncached acl · a8614c2e
      J. R. Okajima authored 
      

When a branch filesystem doesn't cache ACL, aufs should not cache
either.  Until now aufs has never met such fs, but theoretically it
could happen.  Actually, in linux-v5.1-rc1, NFSv3 changed its behaviour
by the commit
	ded52fbe7020 2019-02-20 nfs: fix xfstest generic/099 failed on nfsv3
The commit ded52fbe7020 doesn't "forget" the previous acl though.
Doesn't it mean that the obsoleted acl is kept until NFS's attribute
cache is expired?  I don't know.  I've asked it on LKML, but got no
answer.
Signed-off-by: default avatarJ. R. Okajima <hooanon05g@gmail.com>
(cherry picked from commit e448daa00228186b869356fdef8d98d9f95caf53)
      a8614c2e
  23. 31 Aug, 2019 3 commits
  25. 03 Aug, 2019 2 commits
  27. 07 Jun, 2019 4 commits
    • J. R. Okajima's avatar
      Merge branch 'aufs5.0/00base' into aufs5.0/01modular · 296aa84b
      J. R. Okajima authored
      296aa84b
    • J. R. Okajima's avatar
      aufs: bugfix, no nested RCU for kfree() · 0e79e96a
      J. R. Okajima authored 
      
Signed-off-by: default avatarJ. R. Okajima <hooanon05g@gmail.com>
(cherry picked from commit 4596850541ed7144f7fea951d02f0f6251c4a997)
      0e79e96a
    • J. R. Okajima's avatar
      aufs: possible bugfix, ignore the being freed sbinfo object · 01a1817a
      J. R. Okajima authored 
      

The scenario is very similar to previous commit
"aufs: bugfix, ignore the being freed dynop object".
One exception is that this commit is for sbinfo object which is managed
by kobject (instead of kref).

In order to enter the plink-maintenance mode, users write an ID to
"/proc/fs/aufs/plink_maint" (this path is defined as macros in
include/uapi/linux/aufs_type.h).  If someone else is unmounting the
aufs mount corresponding that ID, then the searcher task may find a
being freed sbinfo object.
The problem and the fix is very similar to previous commit
"aufs: bugfix, ignore the being freed dynop object".
Signed-off-by: default avatarJ. R. Okajima <hooanon05g@gmail.com>
(cherry picked from commit 949b498ae30797b19b9e7ac9b230815f31ffe378)
      01a1817a
    • J. R. Okajima's avatar
      aufs: bugfix, ignore the being freed dynop object · e870e818
      J. R. Okajima authored 
      

Aufs DYNOP (Dynamically customizable FS operations) object is managed by
kref, and when its counter reaches zero, the callback function removes
the object from the internal list which is protected by a spinlock and
then frees the object.
Here there is a small time window between
A: the counter reaches zero, and
B: require the lock to remove the object from the list.
If someone else acquires the lock and searches the list, it may find the
counter-zero'ed object which means the object is being freed.
This commit ignores the object whose counter is already zero.
Reported-and-tested-by: default avatarKirill Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: default avatarJ. R. Okajima <hooanon05g@gmail.com>
(cherry picked from commit b633d7b2635b9615fe294b85257d05008e3747a3)
      e870e818
  29. 27 May, 2019 4 commits
  31. 09 Mar, 2019 3 commits