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. thanks, 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
