On Tue, 2011-11-15 at 19:05 +0200, Kasatkin, Dmitry wrote: > On Tue, Nov 15, 2011 at 4:44 PM, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote: > > On Tue, 2011-11-15 at 16:17 +0200, Kasatkin, Dmitry wrote: > >> On Tue, Nov 15, 2011 at 2:31 PM, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote: > >> > The circular lockdep is caused by allocating the 'iint' for mmapped > >> > files. Originally when an 'iint' was allocated for every inode > >> > in inode_alloc_security(), before the inode was accessible, no > >> > locking was necessary. Commits bc7d2a3e and 196f518 changed this > >> > behavior and allocated the 'iint' on a per need basis, resulting in > >> > the mmap_sem being taken before the i_mutex for mmapped files. > >> > > >> > Possible unsafe locking scenario: > >> > CPU0 CPU1 > >> > ---- ---- > >> > lock(&mm->mmap_sem); > >> > lock(&sb->s_type->i_mutex_key); > >> > lock(&mm->mmap_sem); > >> > lock(&sb->s_type->i_mutex_key); > >> > > >> > This patch adds a new hook ima_file_premmap() to pre-allocate the > >> > iint, preventing the i_mutex being taken after the mmap_sem, and > >> > defines a do_mmap() helper function do_mmap_with_sem(). > >> > > >> > Before making this sort of change throughout, perhaps someone sees > >> > a better option? > >> > > >> > >> Hi, > >> > >> After a bit of thinking I remembered that I have seen ima hooks are > >> called for the same file... > >> i have done call tracing again and found out that. > >> > >> FILE_CHECK is ALWAYS called before FILE_MMAP or BPRM_CHECK. > >> > >> So when 2 above are called, file is already verified.. > >> Indeed, in both cases before mmap or exec, the file is opened with > >> do_filp_open(). > >> > >> Are these completely useless then? > >> FILE_MMAP or BPRM_CHECK > >> > >> - Dmitry > > > > There are a couple of reasons for deferring IMA processing until > > BPRM_CHECK/FILE_MMAP: > > - Defer processing until the file has been locked and won't be modified > > - Different policies can be associated with the different hooks > > > > For example, with the ima_tcb policy, only files opened for read by root > > are measured at file_check, but all files mmapped executable are > > measured at file_mmap. So although a file is opened before it is > > mmapped, we don't know apriori if it will be mmapped. We could allocate > > the iint for all inodes opened for read, but that would kind of defeat > > the purpose of dynamically allocating the iint as needed. > > > > As you are asking for possible alternative solution, > I think I might have one. > > It could possibly done in such away: > > When binaries or executables are opened for mmap or bprm, > kernel sets open_flag |= __FMODE_EXEC; > > ima_file_check() could have additional parameter: op->open_flag > and implementation could selection a function as: > int function = (flag & __FMODE_EXEC) ? BPRM_CHECK : FILE_CHECK; > > IMA policy has the same entries for BPRM_CHECK or FILE_MMAP. > > This can possibly make mmap and bprm hooks redundant. > > - Dmitry As a file can be opened read only and then mmapped executable, it is impossible to know on open, whether that file will be mmapped executable. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
