From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mimi Zohar Subject: Re: [RFC][PATCH] ima: fix lockdep circular locking dependency Date: Tue, 15 Nov 2011 09:44:16 -0500 Message-ID: <1321368273.3780.41.camel@falcor> References: <1321360287-6345-1-git-send-email-zohar@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: linux-security-module@vger.kernel.org, Eric Paris , linux-fsdevel@vger.kernel.org To: "Kasatkin, Dmitry" Return-path: Received: from e8.ny.us.ibm.com ([32.97.182.138]:60239 "EHLO e8.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755843Ab1KOOrZ (ORCPT ); Tue, 15 Nov 2011 09:47:25 -0500 Received: from /spool/local by e8.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 15 Nov 2011 09:47:23 -0500 In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, 2011-11-15 at 16:17 +0200, Kasatkin, Dmitry wrote: > On Tue, Nov 15, 2011 at 2:31 PM, Mimi Zohar 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