From mboxrd@z Thu Jan 1 00:00:00 1970 From: zohar@linux.vnet.ibm.com (Mimi Zohar) Date: Tue, 13 Jun 2017 10:17:45 -0400 Subject: [PATCH 3/4] ima: use existing read file operation method to calculate file hash In-Reply-To: <20170613064658.GB31372@lst.de> References: <1497031364-19949-1-git-send-email-zohar@linux.vnet.ibm.com> <1497031364-19949-4-git-send-email-zohar@linux.vnet.ibm.com> <20170613064658.GB31372@lst.de> Message-ID: <1497363465.21594.395.camel@linux.vnet.ibm.com> To: linux-security-module@vger.kernel.org List-Id: linux-security-module.vger.kernel.org On Tue, 2017-06-13 at 08:46 +0200, Christoph Hellwig wrote: > A strong and a weak NAK on this. For one thing you should not > call ->read for fs code at all - use read_iter where it fits > (it does here) or the kernel_read() helper otherwise. Calling ->read directly is intentional. ?Commit C0430e49b6e7c "ima: introduce ima_kernel_read()" replaced the call to kernel_read with ima_kernel_read(), the non-security checking version of kernel_read(). ?Subsequently, commit e3c4abbfa97e "integrity: define a new function integrity_read_file()" renamed ima_read_file() to integrity_read_file(). > But once again I don't think this is correct - it's a potentially > unsafe default, so please wire up the file systems actually tested > and known to work manually. > > E.g. this does the wrong thing for at least NFS and OCFS2. Both NFS and OCFS define their own specific read_iter(), nfs_file_read() and ocfs2_file_read_iter() respectively. ?As these file systems have not yet been converted to use ->read_integrity, the xfstests fail. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:38636 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753761AbdFMORz (ORCPT ); Tue, 13 Jun 2017 10:17:55 -0400 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v5DEE4s6071380 for ; Tue, 13 Jun 2017 10:17:54 -0400 Received: from e23smtp06.au.ibm.com (e23smtp06.au.ibm.com [202.81.31.148]) by mx0a-001b2d01.pphosted.com with ESMTP id 2b2h4njm7x-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 13 Jun 2017 10:17:54 -0400 Received: from localhost by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 14 Jun 2017 00:17:52 +1000 Subject: Re: [PATCH 3/4] ima: use existing read file operation method to calculate file hash From: Mimi Zohar To: Christoph Hellwig Cc: Al Viro , James Morris , linux-fsdevel@vger.kernel.org, linux-ima-devel@lists.sourceforge.net, linux-security-module@vger.kernel.org Date: Tue, 13 Jun 2017 10:17:45 -0400 In-Reply-To: <20170613064658.GB31372@lst.de> References: <1497031364-19949-1-git-send-email-zohar@linux.vnet.ibm.com> <1497031364-19949-4-git-send-email-zohar@linux.vnet.ibm.com> <20170613064658.GB31372@lst.de> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Message-Id: <1497363465.21594.395.camel@linux.vnet.ibm.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, 2017-06-13 at 08:46 +0200, Christoph Hellwig wrote: > A strong and a weak NAK on this. For one thing you should not > call ->read for fs code at all - use read_iter where it fits > (it does here) or the kernel_read() helper otherwise. Calling ->read directly is intentional.  Commit C0430e49b6e7c "ima: introduce ima_kernel_read()" replaced the call to kernel_read with ima_kernel_read(), the non-security checking version of kernel_read().  Subsequently, commit e3c4abbfa97e "integrity: define a new function integrity_read_file()" renamed ima_read_file() to integrity_read_file(). > But once again I don't think this is correct - it's a potentially > unsafe default, so please wire up the file systems actually tested > and known to work manually. > > E.g. this does the wrong thing for at least NFS and OCFS2. Both NFS and OCFS define their own specific read_iter(), nfs_file_read() and ocfs2_file_read_iter() respectively.  As these file systems have not yet been converted to use ->read_integrity, the xfstests fail. Mimi