Hello Ben,

Ben Hutchings:
> What are these changes supposed to do?
>
> commit aa29a06fd0c6113560f2c5e3e9630fe67b097ba7
> Author: J. R. Okajima <hooano...@yahoo.co.jp>
> Date:   Tue Jun 12 22:08:17 2012 +0900
>
>     aufs: for 3.5-rcN, new au_security_file_mmap() 1/2
>
> commit ad02614f68494af290af54325895091917a6b7c7
> Author: J. R. Okajima <hooano...@yahoo.co.jp>
> Date:   Tue Jun 12 22:08:39 2012 +0900
>
>     aufs: for 3.5-rcN, call au_security_file_mmap() 2/2

Isn't the aa29a06 commit log enough for you?


> An earlier change for 3.5 added this comment, which was removed by the
> second commit above:
>
>        /* todo: the locking order between mmap_sem */
>        /*
>         * err =3D security_mmap_file(h_file, au_prot_conv(vma->vm_flags),
>         *                       au_flag_conv(vma->vm_flags));
>         */

If you looked the another commit (a1d9363), the comments just before
aufs_mmap() (below), and the release mails from me (below), then you might know
what these commits for are.


> Did lockdep report that an mmap_sem and some other lock (needed by an
> LSM) were not taken in the usual order?  If so, moving the
> security_mmap_file() into another thread (workqueue) won't solve the
> possible deadlock, though it may well hide it from lockdep.

Before linux-3.5, aufs called security_file_mmap() called directly.
In 3.5, security_file_mmap() is splitted into two parts as
- security_mmap_addr(), which is called BEFORE acquiring mmap_sem and
  checks the mapping address.
- security_mmap_file(), which is called AFTER acquiring mmap_sem and
  checks the mapping file.

Since security_mmap_file() explicitly expects mmap_sem held, I moved
the security_mmap_file() call to kworker. I don't know whether
security_mmap_file() actulally causes a deadlock or not.
This is very dirty approach (as I wrote), but it is effective. Because
mmap_sem is an object per task (process or thread), the deadlock will
never happen. The mmap_sem for the process is held, but the mmap_sem for
the kworker is not (and it is unrelated).
Don't you agree?


(aa29a06 commit)
----------------------------------------------------------------------
    A bad approach in order to just call security_file_mmap() for the
    real file on branch fs from outside of 'mmap_sem'-protected region.
    It may be meaningless since it is done in kworker context.
    I really don't like this approach but I don't have another approach to
    call security_file_mmap().

(the comments just before aufs_mmap())
----------------------------------------------------------------------
/*
 * The locking order around current->mmap_sem.
 * - in most and regular cases
 *   file I/O syscall -- aufs_read() or something
 *      -- si_rwsem for read -- mmap_sem
 *      (Note that [fdi]i_rwsem are released before mmap_sem).
 * - in mmap case
 *   mmap(2) -- mmap_sem -- aufs_mmap() -- si_rwsem for read -- [fdi]i_rwsem
 * This AB-BA order is definitly bad, but is not a problem since "si_rwsem for
 * read" allows muliple processes to acquire it and [fdi]i_rwsem are not held in
 * file I/O. Aufs needs to stop lockdep in aufs_mmap() though.
 * It means that when aufs acquires si_rwsem for write, the process should never
 * acquire mmap_sem.
 *
 * Actually aufs_readdir() holds [fdi]i_rwsem before mmap_sem, but this is not a
 * problem either since any directory is not able to be mmap-ed.
 * The similar scenario is applied to aufs_readlink() too.
 */

(mail on 11 Jun)
----------------------------------------------------------------------
From: sf...@users.sourceforge.net
To: aufs-users@lists.sourceforge.net
Subject: aufs3 GIT release
Date: Mon, 11 Jun 2012 15:29:49 +0900

        :::
- the aufs work for linux-3.5-rcN is not completed yet.
        :::
      aufs: for 3.5-rcN, security_mmap_file()
        :::

(mail on 18 Jun)
----------------------------------------------------------------------
From: sf...@users.sourceforge.net
To: aufs-users@lists.sourceforge.net
Subject: aufs3 GIT release
Date: Mon, 18 Jun 2012 17:15:51 +0900

- the aufs work for linux-3.5-rc2 completes, and this version of
        :::
      aufs: for 3.5-rcN, new au_security_file_mmap() 1/2
      aufs: for 3.5-rcN, call au_security_file_mmap() 2/2
        :::
----------------------------------------------------------------------

J. R. Okajima

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/

Reply via email to