From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-wr0-f169.google.com ([209.85.128.169]:36228 "EHLO
        mail-wr0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753791AbdEJPzz (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Wed, 10 May 2017 11:55:55 -0400
Received: by mail-wr0-f169.google.com with SMTP id l50so48866732wrc.3
        for <linux-fsdevel@vger.kernel.org>; Wed, 10 May 2017 08:55:54 -0700 (PDT)
Subject: Re: [PATCH] security/ima: use fs method to read integrity data
To: Christoph Hellwig <hch@lst.de>, Boaz Harrosh <boaz@plexistor.com>
References: <20170510064507.1764-1-hch@lst.de>
 <20170510064507.1764-2-hch@lst.de>
 <a552827e-922d-0b28-2919-90c780a8a414@plexistor.com>
 <20170510132359.GA22549@lst.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
        Mimi Zohar <zohar@linux.vnet.ibm.com>,
        linux-fsdevel@vger.kernel.org,
        linux-ima-devel@lists.sourceforge.net,
        linux-security-module@vger.kernel.org
From: Boaz Harrosh <boaz@plexistor.com>
Message-ID: <84bba69e-64a6-432d-bf27-54eb7669cea8@plexistor.com>
Date: Wed, 10 May 2017 18:55:51 +0300
MIME-Version: 1.0
In-Reply-To: <20170510132359.GA22549@lst.de>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On 05/10/2017 04:24 PM, Christoph Hellwig wrote:
> On Wed, May 10, 2017 at 03:20:41PM +0300, Boaz Harrosh wrote:
>> Would you not want to call ->read_iter() in the NULL case
>> and have all FSs supported as today?
> 
> As IMA has particular requirements on the fs (e.g. that it can
> read with i_rwsem held as seen in this patch, or useful i_version
> which only the file systems converted in this patch do), having
> an explicit opt-in seems much safer.  This optional method is
> a very easy way to provide this opt-in behavior.
> 

>> +	if (!file->f_op->integrity_read)
>> +		return -EBADF;
> 
> Would you not want to call ->read_iter() in the NULL case
> and have all FSs supported as today?
> 
> Thanks
> Boaz
> 
>>  
>> -	old_fs = get_fs();
>> -	set_fs(get_ds());
>> -	ret = __vfs_read(file, buf, count, &offset);
>> -	set_fs(old_fs);

If you look here above, it used to call __vfs_read regardless
of i_version and "opt-in" so it looks like a regression for
all those FSs that used to work.

So I did not understand, but I guess you are right opt-in seems much
safer.

Thanks
Boaz

>> +	init_sync_kiocb(&kiocb, file);
>> +	kiocb.ki_pos = offset;
>> +	iov_iter_init(&iter, READ | ITER_KVEC, &iov, 1, count);