From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752093AbeBTTTX (ORCPT ); Tue, 20 Feb 2018 14:19:23 -0500 Received: from mail.kernel.org ([198.145.29.99]:58284 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751854AbeBTTTV (ORCPT ); Tue, 20 Feb 2018 14:19:21 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0762621104 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=luto@kernel.org Subject: Re: [PATCH 1/2] fs/efivarfs: restrict inode permissions To: Joe Konno , linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org Cc: ard.biesheuvel@linaro.org, matthew.garrett@nebula.com, jk@ozlabs.org, ak@linux.intel.com, tony.luck@intel.com References: <20180215182208.35003-1-joe.konno@linux.intel.com> <20180215182208.35003-2-joe.konno@linux.intel.com> From: Andy Lutomirski Message-ID: <6680a760-eb30-4daf-2dad-a9628f1c15a8@kernel.org> Date: Tue, 20 Feb 2018 11:18:57 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180215182208.35003-2-joe.konno@linux.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/15/2018 10:22 AM, Joe Konno wrote: > From: Joe Konno > > Efivarfs nodes are created with group and world readable permissions. > Reading certain EFI variables trigger SMIs. So, this is a potential DoS > surface. > > Make permissions more restrictive-- only the owner may read or write to > created inodes. > > Suggested-by: Andi Kleen > Reported-by: Tony Luck > Cc: Ard Biesheuvel > Cc: Matthew Garrett > Cc: Jeremy Kerr > Cc: linux-efi@vger.kernel.org > Cc: stable@vger.kernel.org > Signed-off-by: Joe Konno The discussion in this thread has gone on too long, so: Acked-by: Andy Lutomirski And yes, this patch will break a couple of minor usecases, but IMO those usecases deserve to break. > --- > fs/efivarfs/super.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c > index 5b68e4294faa..ca98c4e31eb7 100644 > --- a/fs/efivarfs/super.c > +++ b/fs/efivarfs/super.c > @@ -145,7 +145,7 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor, > > name[len + EFI_VARIABLE_GUID_LEN+1] = '\0'; > > - inode = efivarfs_get_inode(sb, d_inode(root), S_IFREG | 0644, 0, > + inode = efivarfs_get_inode(sb, d_inode(root), S_IFREG | 0600, 0, > is_removable); > if (!inode) > goto fail_name; > @@ -207,7 +207,7 @@ static int efivarfs_fill_super(struct super_block *sb, void *data, int silent) > sb->s_d_op = &efivarfs_d_ops; > sb->s_time_gran = 1; > > - inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0755, 0, true); > + inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0700, 0, true); > if (!inode) > return -ENOMEM; > inode->i_op = &efivarfs_dir_inode_operations; >