Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754717AbdIRJTg (ORCPT ); Mon, 18 Sep 2017 05:19:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44044 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752586AbdIRJTe (ORCPT ); Mon, 18 Sep 2017 05:19:34 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 553D47E420 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=swhiteho@redhat.com Subject: Re: [PATCH 3/3] ima: use fs method to read integrity data (updated patch description) To: Al Viro , Linus Torvalds References: <1505451494-30228-1-git-send-email-zohar@linux.vnet.ibm.com> <1505451494-30228-4-git-send-email-zohar@linux.vnet.ibm.com> <1505507142.4200.103.camel@linux.vnet.ibm.com> <20170917151757.GA14262@infradead.org> <1505664935.4200.191.camel@linux.vnet.ibm.com> <20170917163828.GE5426@ZenIV.linux.org.uk> Cc: Mimi Zohar , Christoph Hellwig , LSM List , Christoph Hellwig , linux-ima-devel@lists.sourceforge.net, James Morris , Linux Kernel Mailing List , Matthew Garrett , Jan Kara , "Theodore Ts'o" , Andreas Dilger , Jaegeuk Kim , Chao Yu , Bob Peterson , David Woodhouse , Dave Kleikamp , Ryusuke Konishi , Mark Fasheh , Joel Becker , Richard Weinberger , "Darrick J. Wong" , Hugh Dickins , Chris Mason From: Steven Whitehouse Message-ID: <517c83a6-d7c5-9638-ebaa-52800ca0962c@redhat.com> Date: Mon, 18 Sep 2017 10:19:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170917163828.GE5426@ZenIV.linux.org.uk> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 18 Sep 2017 09:19:34 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1912 Lines: 40 Hi, On 17/09/17 17:38, Al Viro wrote: > On Sun, Sep 17, 2017 at 09:34:01AM -0700, Linus Torvalds wrote: >> Now, I suspect most (all?) do, but that's a historical artifact rather >> than "design". In particular, the VFS layer used to do the locking for >> the filesystems, to guarantee the POSIX requirements (POSIX requires >> that writes be seen atomically). >> >> But that lock was pushed down into the filesystems, since some >> filesystems really wanted to have parallel writes (particularly for >> direct IO, where that POSIX serialization requirement doesn't exist). >> >> That's all many years ago, though. New filesystems are likely to have >> copied the pattern from old ones, but even then.. >> >> Also, it's worth noting that "inode->i_rwlock" isn't even well-defined >> as a lock. You can have the question of *which* inode gets talked >> about when you have things like eoverlayfs etc. Normally it would be >> obvious, but sometimes you'd use "file->f_mapping->host" (which is the >> same thing in the simple cases), and sometimes it really wouldn't be >> obvious at all.. >> >> So... I'm really not at all convinced that i_rwsem is sensible. It's >> one of those things that are "mostly right for the simple cases", >> but... > The thing pretty much common to all of them is that write() might need > to modify permissions (suid removal), which brings ->i_rwsem in one > way or another - notify_change() needs that held... For GFS2, if we are to hold the inode info constant while it is checked, we would need to take a glock (read lock in this case) across the relevant operations. The glock will be happy under i_rwlock, since we have a lock ordering that takes local locks ahead of cluster locks. I've not dug into this enough to figure out whether the current proposal will allow this to work with GFS2 though. Does IMA cache the results from the ->read_integrity() operation? Steve.