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 > thanks, > > Mimi > > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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
