From patchwork Tue Sep 5 22:35:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ross Zwisler X-Patchwork-Id: 9939741 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0A2FD604D3 for ; Tue, 5 Sep 2017 22:36:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EF63828A1C for ; Tue, 5 Sep 2017 22:36:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E40A928A23; Tue, 5 Sep 2017 22:36:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A597428A1C for ; Tue, 5 Sep 2017 22:36:25 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 1764221E95E05; Tue, 5 Sep 2017 15:33:31 -0700 (PDT) X-Original-To: linux-nvdimm@lists.01.org Delivered-To: linux-nvdimm@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8F78621E3EA73 for ; Tue, 5 Sep 2017 15:33:29 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Sep 2017 15:36:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,481,1498546800"; d="scan'208";a="308314895" Received: from theros.lm.intel.com ([10.232.112.77]) by fmsmga004.fm.intel.com with ESMTP; 05 Sep 2017 15:36:18 -0700 From: Ross Zwisler To: Andrew Morton , linux-kernel@vger.kernel.org Subject: [PATCH 8/9] ext4: add sanity check for encryption + DAX Date: Tue, 5 Sep 2017 16:35:40 -0600 Message-Id: <20170905223541.20594-9-ross.zwisler@linux.intel.com> X-Mailer: git-send-email 2.9.5 In-Reply-To: <20170905223541.20594-1-ross.zwisler@linux.intel.com> References: <20170905223541.20594-1-ross.zwisler@linux.intel.com> X-BeenThere: linux-nvdimm@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Linux-nvdimm developer list." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Theodore Ts'o , "Darrick J. Wong" , Dave Chinner , linux-nvdimm@lists.01.org, stable@vger.kernel.org, linux-xfs@vger.kernel.org, Andreas Dilger , Jan Kara , linux-ext4@vger.kernel.org, Christoph Hellwig MIME-Version: 1.0 Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" X-Virus-Scanned: ClamAV using ClamSMTP We prevent DAX from being used on inodes which are using ext4's built in encryption via a check in ext4_should_use_dax(). We do have what appears to be an unsafe transition of S_DAX in ext4_set_context(), though, where S_DAX can get disabled without us doing a proper writeback + invalidate. I actually think we are safe in this case because of the following: 1) You can't encrypt an existing file. Encryption can only be set on an empty directory, with new inodes in that directory being created with encryption turned on, so I don't think it's possible to turn encryption on for a file that has open DAX mmaps or outstanding I/Os. 2) There is no way to turn encryption off on a given file. Once an inode is encrypted, it stays encrypted for the life of that inode, so we don't have to worry about the case where we turn encryption off and S_DAX suddenly turns on. 3) The only way we end up in ext4_set_context() to turn on encryption is when we are creating a new file in the encrypted directory. This happens as part of ext4_create() before the inode has been allowed to do any I/O. Here's the call tree: ext4_create() __ext4_new_inode() ext4_set_inode_flags() // sets S_DAX fscrypt_inherit_context() fscrypt_get_encryption_info(); ext4_set_context() // sets EXT4_INODE_ENCRYPT, clears S_DAX So, I actually think it's safe to transition S_DAX in ext4_set_context() without any locking, writebacks or invalidations. I've added a WARN_ON_ONCE() sanity check to make sure that we are notified if we ever encounter a case where we are encrypting an inode that already has data, in which case we need to add code to safely transition S_DAX. Signed-off-by: Ross Zwisler CC: stable@vger.kernel.org --- fs/ext4/super.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index d549dfb..6604a18 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1159,6 +1159,9 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len, if (inode->i_ino == EXT4_ROOT_INO) return -EPERM; + if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode))) + return -EINVAL; + res = ext4_convert_inline_data(inode); if (res) return res;