Message ID | 20221106210744.603240-3-nayna@linux.ibm.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp1675778wru; Sun, 6 Nov 2022 13:09:23 -0800 (PST) X-Google-Smtp-Source: AMsMyM6bEOFrXQnvIno0d83rJC8vZidjJt7lTNKn3s93fng6xK4/M1BjIelBZm10UtoRfGdrCrQi X-Received: by 2002:a17:902:7c95:b0:188:5e9a:c643 with SMTP id y21-20020a1709027c9500b001885e9ac643mr17932678pll.8.1667768963259; Sun, 06 Nov 2022 13:09:23 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1667768963; cv=none; d=google.com; s=arc-20160816; b=jr0j/qp7AomL0zEEu8CRU1IXVbUwIBGG5+QkwPprULP4ycw047Wa+TZVYsOurYA8GG 9MQGKvAt4DO4jthTiSjtg84YAtIgfCAzZdEVsMkzj6S8ipSyXC+Skrb+q/74Xl1Uvjv6 w8g3kvScyd2tkb3Mw9336jfhlkJMuYpEkSMmJkF7rfoomiIO4aOdLUeV0XNSeLRlKakL /CjQySC6ekiclfAxI1Kjh6fYkUgXdCnYBoyPSZf73xSzYk+11ZVyymiVdL39XFUWGydA w1aVqxpwJ0WS4b3EEgXXlm37Ph2QgIXz/T3uYatqFg8KSMTErZzg1Q75I74DaV358DbZ o6rg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=Dqgb6V30u6xH6F66DO7gRqyHAW/t9HZRw8bgLYM5ckw=; b=DTlZt4lw4mbWOIXkXR2z5xDPBGf9rbNT2Y0Zh1t2U+Epjggyo4cqyWY9Fq6zrk7Z9x ZOJRZH1wZv9egwP1LgJpVMayQN5Tzhso1CVucCFJ9sS3wPnW7pOp0xzhLz+qbZYbFa7u JiqPMCihgGQTVqA6M2KuUp7XnsM8/LmJ2xgHc/rOkIZyjO5Byfbi5Q57DK9U6K6/OMA8 ZpZR+tbIs3TorxRz+ip6QDbgzyCBj+zCzRCBm0TXGJrb9TTPzFx0A52PaRMSAFzkUHwe TZMMfcs3uTkTJ/+4gLIwhXQun0qFlDpOZRU1E61+f3NN9M1yknekeDt+ByX0+vRvw5sZ rqsQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ibm.com header.s=pp1 header.b=kbNAHBCE; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=ibm.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b10-20020a170903228a00b001780ba6c694si8821107plh.35.2022.11.06.13.09.10; Sun, 06 Nov 2022 13:09:23 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@ibm.com header.s=pp1 header.b=kbNAHBCE; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=ibm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230210AbiKFVIy (ORCPT <rfc822;hjfbswb@gmail.com> + 99 others); Sun, 6 Nov 2022 16:08:54 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34664 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230171AbiKFVIp (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 6 Nov 2022 16:08:45 -0500 Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 28335E94; Sun, 6 Nov 2022 13:08:44 -0800 (PST) Received: from pps.filterd (m0187473.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 2A6IUAFO011922; Sun, 6 Nov 2022 21:08:18 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=pp1; bh=Dqgb6V30u6xH6F66DO7gRqyHAW/t9HZRw8bgLYM5ckw=; b=kbNAHBCErfLINFOuOiMPnoow6F32B88f+tjTazQiuPItwxFaLqOFg4jGjJBGnleBifxS fGxv0C7G85ivViME9xVq+b0qRDoa4RMlSaTj71Gug7Z0ct/3GK3AxX57IzWt6CiDEl20 HmXcAkK8oqYvdfcNf7NaoqokjRghSo1ts3ITGhUfKnSGcIpjy/hu4vzlTW70gLjTlnzl Gp0unEnMzEQ7XXVSTfK35kuWILVLLwtzGXCppQsYYx3+NGxfW22T5CakTN4t+nGP0nf1 mPcPyV25jyZLY8iiuJJMh7ssdwpOf0oL9vLjfuoZ59RdGJjvtivgkzsKMWGUMbv04O/P oQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3kp1tecujy-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 06 Nov 2022 21:08:17 +0000 Received: from m0187473.ppops.net (m0187473.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 2A6KtCZS004297; Sun, 6 Nov 2022 21:08:17 GMT Received: from ppma02fra.de.ibm.com (47.49.7a9f.ip4.static.sl-reverse.com [159.122.73.71]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3kp1tecuje-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 06 Nov 2022 21:08:16 +0000 Received: from pps.filterd (ppma02fra.de.ibm.com [127.0.0.1]) by ppma02fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 2A6L5621016614; Sun, 6 Nov 2022 21:08:14 GMT Received: from b06cxnps4076.portsmouth.uk.ibm.com (d06relay13.portsmouth.uk.ibm.com [9.149.109.198]) by ppma02fra.de.ibm.com with ESMTP id 3kngpgh73j-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 06 Nov 2022 21:08:14 +0000 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps4076.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 2A6L8BB837683662 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sun, 6 Nov 2022 21:08:11 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1824A11C050; Sun, 6 Nov 2022 21:08:11 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3FDAB11C04C; Sun, 6 Nov 2022 21:08:07 +0000 (GMT) Received: from li-4b5937cc-25c4-11b2-a85c-cea3a66903e4.ibm.com.com (unknown [9.211.78.124]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Sun, 6 Nov 2022 21:08:07 +0000 (GMT) From: Nayna Jain <nayna@linux.ibm.com> To: linuxppc-dev@lists.ozlabs.org, linux-fsdevel@vger.kernel.org Cc: linux-efi@vger.kernel.org, linux-security-module <linux-security-module@vger.kernel.org>, linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Michael Ellerman <mpe@ellerman.id.au>, npiggin@gmail.com, christophe.leroy@csgroup.eu, Dov Murik <dovmurik@linux.ibm.com>, George Wilson <gcwilson@linux.ibm.com>, Matthew Garrett <mjg59@srcf.ucam.org>, Dave Hansen <dave.hansen@intel.com>, Benjamin Herrenschmidt <benh@kernel.crashing.org>, Paul Mackerras <paulus@samba.org>, Russell Currey <ruscur@russell.cc>, Andrew Donnellan <ajd@linux.ibm.com>, Stefan Berger <stefanb@linux.ibm.com>, Nayna Jain <nayna@linux.ibm.com> Subject: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs Date: Sun, 6 Nov 2022 16:07:42 -0500 Message-Id: <20221106210744.603240-3-nayna@linux.ibm.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20221106210744.603240-1-nayna@linux.ibm.com> References: <20221106210744.603240-1-nayna@linux.ibm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: K3_cqkK3xbQIHy73A5pjMwCCpwqxa3JZ X-Proofpoint-ORIG-GUID: k5ARmYELm2Mnq7-NXOeNpwZ_BKzPCB6- X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-11-06_14,2022-11-03_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 impostorscore=0 mlxscore=0 lowpriorityscore=0 spamscore=0 malwarescore=0 mlxlogscore=999 suspectscore=0 phishscore=0 priorityscore=1501 adultscore=0 bulkscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000 definitions=main-2211060188 X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748782508674425008?= X-GMAIL-MSGID: =?utf-8?q?1748782508674425008?= |
Series |
powerpc/pseries: expose firmware security variables via filesystem
|
|
Commit Message
Nayna Jain
Nov. 6, 2022, 9:07 p.m. UTC
securityfs is meant for Linux security subsystems to expose policies/logs
or any other information. However, there are various firmware security
features which expose their variables for user management via the kernel.
There is currently no single place to expose these variables. Different
platforms use sysfs/platform specific filesystem(efivarfs)/securityfs
interface as they find it appropriate. Thus, there is a gap in kernel
interfaces to expose variables for security features.
Define a firmware security filesystem (fwsecurityfs) to be used by
security features enabled by the firmware. These variables are platform
specific. This filesystem provides platforms a way to implement their
own underlying semantics by defining own inode and file operations.
Similar to securityfs, the firmware security filesystem is recommended
to be exposed on a well known mount point /sys/firmware/security.
Platforms can define their own directory or file structure under this path.
Example:
# mount -t fwsecurityfs fwsecurityfs /sys/firmware/security
# cd /sys/firmware/security/
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
fs/Kconfig | 1 +
fs/Makefile | 1 +
fs/fwsecurityfs/Kconfig | 14 ++
fs/fwsecurityfs/Makefile | 10 ++
fs/fwsecurityfs/super.c | 263 +++++++++++++++++++++++++++++++++++
include/linux/fwsecurityfs.h | 29 ++++
include/uapi/linux/magic.h | 1 +
7 files changed, 319 insertions(+)
create mode 100644 fs/fwsecurityfs/Kconfig
create mode 100644 fs/fwsecurityfs/Makefile
create mode 100644 fs/fwsecurityfs/super.c
create mode 100644 include/linux/fwsecurityfs.h
Comments
Hi Nayna, I love your patch! Yet something to improve: [auto build test ERROR on powerpc/next] [also build test ERROR on linus/master v6.1-rc4 next-20221104] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Nayna-Jain/powerpc-pseries-expose-firmware-security-variables-via-filesystem/20221107-053231 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next patch link: https://lore.kernel.org/r/20221106210744.603240-3-nayna%40linux.ibm.com patch subject: [PATCH 2/4] fs: define a firmware security filesystem named fwsecurityfs config: x86_64-allyesconfig compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/a0d08494d55c4b958e23479a4eb55c144b719d95 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Nayna-Jain/powerpc-pseries-expose-firmware-security-variables-via-filesystem/20221107-053231 git checkout a0d08494d55c4b958e23479a4eb55c144b719d95 # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): fs/fwsecurityfs/super.c: In function 'fwsecurityfs_init': >> fs/fwsecurityfs/super.c:248:14: error: implicit declaration of function 'sysfs_create_mount_point' [-Werror=implicit-function-declaration] 248 | rc = sysfs_create_mount_point(firmware_kobj, "security"); | ^~~~~~~~~~~~~~~~~~~~~~~~ >> fs/fwsecurityfs/super.c:248:39: error: 'firmware_kobj' undeclared (first use in this function) 248 | rc = sysfs_create_mount_point(firmware_kobj, "security"); | ^~~~~~~~~~~~~ fs/fwsecurityfs/super.c:248:39: note: each undeclared identifier is reported only once for each function it appears in >> fs/fwsecurityfs/super.c:254:17: error: implicit declaration of function 'sysfs_remove_mount_point' [-Werror=implicit-function-declaration] 254 | sysfs_remove_mount_point(firmware_kobj, "security"); | ^~~~~~~~~~~~~~~~~~~~~~~~ fs/fwsecurityfs/super.c: At top level: >> fs/fwsecurityfs/super.c:261:20: error: expected declaration specifiers or '...' before string constant 261 | MODULE_DESCRIPTION("Firmware Security Filesystem"); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ fs/fwsecurityfs/super.c:262:15: error: expected declaration specifiers or '...' before string constant 262 | MODULE_AUTHOR("Nayna Jain"); | ^~~~~~~~~~~~ fs/fwsecurityfs/super.c:263:16: error: expected declaration specifiers or '...' before string constant 263 | MODULE_LICENSE("GPL"); | ^~~~~ cc1: some warnings being treated as errors vim +/sysfs_create_mount_point +248 fs/fwsecurityfs/super.c 243 244 static int __init fwsecurityfs_init(void) 245 { 246 int rc; 247 > 248 rc = sysfs_create_mount_point(firmware_kobj, "security"); 249 if (rc) 250 return rc; 251 252 rc = register_filesystem(&fs_type); 253 if (rc) { > 254 sysfs_remove_mount_point(firmware_kobj, "security"); 255 return rc; 256 } 257 258 return 0; 259 } 260 core_initcall(fwsecurityfs_init); > 261 MODULE_DESCRIPTION("Firmware Security Filesystem");
On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: > securityfs is meant for Linux security subsystems to expose policies/logs > or any other information. However, there are various firmware security > features which expose their variables for user management via the kernel. > There is currently no single place to expose these variables. Different > platforms use sysfs/platform specific filesystem(efivarfs)/securityfs > interface as they find it appropriate. Thus, there is a gap in kernel > interfaces to expose variables for security features. > > Define a firmware security filesystem (fwsecurityfs) to be used by > security features enabled by the firmware. These variables are platform > specific. This filesystem provides platforms a way to implement their > own underlying semantics by defining own inode and file operations. > > Similar to securityfs, the firmware security filesystem is recommended > to be exposed on a well known mount point /sys/firmware/security. > Platforms can define their own directory or file structure under this path. > > Example: > > # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security Why not juset use securityfs in /sys/security/firmware/ instead? Then you don't have to create a new filesystem and convince userspace to mount it in a specific location? thanks, greg k-h
On 11/9/22 08:46, Greg Kroah-Hartman wrote: > On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: >> securityfs is meant for Linux security subsystems to expose policies/logs >> or any other information. However, there are various firmware security >> features which expose their variables for user management via the kernel. >> There is currently no single place to expose these variables. Different >> platforms use sysfs/platform specific filesystem(efivarfs)/securityfs >> interface as they find it appropriate. Thus, there is a gap in kernel >> interfaces to expose variables for security features. >> >> Define a firmware security filesystem (fwsecurityfs) to be used by >> security features enabled by the firmware. These variables are platform >> specific. This filesystem provides platforms a way to implement their >> own underlying semantics by defining own inode and file operations. >> >> Similar to securityfs, the firmware security filesystem is recommended >> to be exposed on a well known mount point /sys/firmware/security. >> Platforms can define their own directory or file structure under this path. >> >> Example: >> >> # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security > Why not juset use securityfs in /sys/security/firmware/ instead? Then > you don't have to create a new filesystem and convince userspace to > mount it in a specific location? From man 5 sysfs page: /sys/firmware: This subdirectory contains interfaces for viewing and manipulating firmware-specific objects and attributes. /sys/kernel: This subdirectory contains various files and subdirectories that provide information about the running kernel. The security variables which are being exposed via fwsecurityfs are managed by firmware, stored in firmware managed space and also often consumed by firmware for enabling various security features. From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of securityfs(/sys/kernel/security) is to provide a common place for all kernel LSMs. The idea of fwsecurityfs(/sys/firmware/security) is to similarly provide a common place for all firmware security objects. /sys/firmware already exists. The patch now defines a new /security directory in it for firmware security features. Using /sys/kernel/security would mean scattering firmware objects in multiple places and confusing the purpose of /sys/kernel and /sys/firmware. Even though fwsecurityfs code is based on securityfs, since the two filesystems expose different types of objects and have different requirements, there are distinctions: 1. fwsecurityfs lets users create files in userspace, securityfs only allows kernel subsystems to create files. 2. firmware and kernel objects may have different requirements. For example, consideration of namespacing. As per my understanding, namespacing is applied to kernel resources and not firmware resources. That's why it makes sense to add support for namespacing in securityfs, but we concluded that fwsecurityfs currently doesn't need it. Another but similar example of it is: TPM space, which is exposed from hardware. For containers, the TPM would be made as virtual/software TPM. Similarly for firmware space for containers, it would have to be something virtualized/software version of it. 3. firmware objects are persistent and read at boot time by interaction with firmware, unlike kernel objects which are not persistent. For a more detailed explanation refer to the LSS-NA 2022 "PowerVM Platform Keystore - Securing Linux Credentials Locally" talk and slides[1]. The link to previously posted RFC version is [2]. [1] https://static.sched.com/hosted_files/lssna2022/25/NaynaJain_PowerVM_PlatformKeyStore_SecuringLinuxCredentialsLocally.pdf [2] https://lore.kernel.org/linuxppc-dev/YrQqPhi4+jHZ1WJc@kroah.com/ Thanks & Regards, - Nayna > > thanks, > > greg k-h
On Wed, Nov 09, 2022 at 03:10:37PM -0500, Nayna wrote: > > On 11/9/22 08:46, Greg Kroah-Hartman wrote: > > On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: > > > securityfs is meant for Linux security subsystems to expose policies/logs > > > or any other information. However, there are various firmware security > > > features which expose their variables for user management via the kernel. > > > There is currently no single place to expose these variables. Different > > > platforms use sysfs/platform specific filesystem(efivarfs)/securityfs > > > interface as they find it appropriate. Thus, there is a gap in kernel > > > interfaces to expose variables for security features. > > > > > > Define a firmware security filesystem (fwsecurityfs) to be used by > > > security features enabled by the firmware. These variables are platform > > > specific. This filesystem provides platforms a way to implement their > > > own underlying semantics by defining own inode and file operations. > > > > > > Similar to securityfs, the firmware security filesystem is recommended > > > to be exposed on a well known mount point /sys/firmware/security. > > > Platforms can define their own directory or file structure under this path. > > > > > > Example: > > > > > > # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security > > Why not juset use securityfs in /sys/security/firmware/ instead? Then > > you don't have to create a new filesystem and convince userspace to > > mount it in a specific location? > > From man 5 sysfs page: > > /sys/firmware: This subdirectory contains interfaces for viewing and > manipulating firmware-specific objects and attributes. > > /sys/kernel: This subdirectory contains various files and subdirectories > that provide information about the running kernel. > > The security variables which are being exposed via fwsecurityfs are managed > by firmware, stored in firmware managed space and also often consumed by > firmware for enabling various security features. Ok, then just use the normal sysfs interface for /sys/firmware, why do you need a whole new filesystem type? > From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of > securityfs(/sys/kernel/security) is to provide a common place for all kernel > LSMs. The idea of > fwsecurityfs(/sys/firmware/security) is to similarly provide a common place > for all firmware security objects. > > /sys/firmware already exists. The patch now defines a new /security > directory in it for firmware security features. Using /sys/kernel/security > would mean scattering firmware objects in multiple places and confusing the > purpose of /sys/kernel and /sys/firmware. sysfs is confusing already, no problem with making it more confusing :) Just document where you add things and all should be fine. > Even though fwsecurityfs code is based on securityfs, since the two > filesystems expose different types of objects and have different > requirements, there are distinctions: > > 1. fwsecurityfs lets users create files in userspace, securityfs only allows > kernel subsystems to create files. Wait, why would a user ever create a file in this filesystem? If you need that, why not use configfs? That's what that is for, right? > 2. firmware and kernel objects may have different requirements. For example, > consideration of namespacing. As per my understanding, namespacing is > applied to kernel resources and not firmware resources. That's why it makes > sense to add support for namespacing in securityfs, but we concluded that > fwsecurityfs currently doesn't need it. Another but similar example of it > is: TPM space, which is exposed from hardware. For containers, the TPM would > be made as virtual/software TPM. Similarly for firmware space for > containers, it would have to be something virtualized/software version of > it. I do not understand, sorry. What does namespaces have to do with this? sysfs can already handle namespaces just fine, why not use that? > 3. firmware objects are persistent and read at boot time by interaction with > firmware, unlike kernel objects which are not persistent. That doesn't matter, sysfs exports what the hardware provides, and that might persist over boot. So I don't see why a new filesystem is needed. You didn't explain why sysfs, or securitfs (except for the location in the tree) does not work at all for your needs. The location really doesn't matter all that much as you are creating a brand new location anyway so we can just declare "this is where this stuff goes" and be ok. And again, how are you going to get all Linux distros to now mount your new filesystem? thanks, greg k-h
On 11/10/22 04:58, Greg Kroah-Hartman wrote: > On Wed, Nov 09, 2022 at 03:10:37PM -0500, Nayna wrote: >> On 11/9/22 08:46, Greg Kroah-Hartman wrote: >>> On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: >>>> securityfs is meant for Linux security subsystems to expose policies/logs >>>> or any other information. However, there are various firmware security >>>> features which expose their variables for user management via the kernel. >>>> There is currently no single place to expose these variables. Different >>>> platforms use sysfs/platform specific filesystem(efivarfs)/securityfs >>>> interface as they find it appropriate. Thus, there is a gap in kernel >>>> interfaces to expose variables for security features. >>>> >>>> Define a firmware security filesystem (fwsecurityfs) to be used by >>>> security features enabled by the firmware. These variables are platform >>>> specific. This filesystem provides platforms a way to implement their >>>> own underlying semantics by defining own inode and file operations. >>>> >>>> Similar to securityfs, the firmware security filesystem is recommended >>>> to be exposed on a well known mount point /sys/firmware/security. >>>> Platforms can define their own directory or file structure under this path. >>>> >>>> Example: >>>> >>>> # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security >>> Why not juset use securityfs in /sys/security/firmware/ instead? Then >>> you don't have to create a new filesystem and convince userspace to >>> mount it in a specific location? >> From man 5 sysfs page: >> >> /sys/firmware: This subdirectory contains interfaces for viewing and >> manipulating firmware-specific objects and attributes. >> >> /sys/kernel: This subdirectory contains various files and subdirectories >> that provide information about the running kernel. >> >> The security variables which are being exposed via fwsecurityfs are managed >> by firmware, stored in firmware managed space and also often consumed by >> firmware for enabling various security features. > Ok, then just use the normal sysfs interface for /sys/firmware, why do > you need a whole new filesystem type? > >> From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of >> securityfs(/sys/kernel/security) is to provide a common place for all kernel >> LSMs. The idea of >> fwsecurityfs(/sys/firmware/security) is to similarly provide a common place >> for all firmware security objects. >> >> /sys/firmware already exists. The patch now defines a new /security >> directory in it for firmware security features. Using /sys/kernel/security >> would mean scattering firmware objects in multiple places and confusing the >> purpose of /sys/kernel and /sys/firmware. > sysfs is confusing already, no problem with making it more confusing :) > > Just document where you add things and all should be fine. > >> Even though fwsecurityfs code is based on securityfs, since the two >> filesystems expose different types of objects and have different >> requirements, there are distinctions: >> >> 1. fwsecurityfs lets users create files in userspace, securityfs only allows >> kernel subsystems to create files. > Wait, why would a user ever create a file in this filesystem? If you > need that, why not use configfs? That's what that is for, right? The purpose of fwsecurityfs is not to expose configuration items but rather security objects used for firmware security features. I think these are more comparable to EFI variables, which are exposed via an EFI-specific filesystem, efivarfs, rather than configfs. > >> 2. firmware and kernel objects may have different requirements. For example, >> consideration of namespacing. As per my understanding, namespacing is >> applied to kernel resources and not firmware resources. That's why it makes >> sense to add support for namespacing in securityfs, but we concluded that >> fwsecurityfs currently doesn't need it. Another but similar example of it >> is: TPM space, which is exposed from hardware. For containers, the TPM would >> be made as virtual/software TPM. Similarly for firmware space for >> containers, it would have to be something virtualized/software version of >> it. > I do not understand, sorry. What does namespaces have to do with this? > sysfs can already handle namespaces just fine, why not use that? Firmware objects are not namespaced. I mentioned it here as an example of the difference between firmware and kernel objects. It is also in response to the feedback from James Bottomley in RFC v2 [https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38adb59a66dcae4c59b.camel@HansenPartnership.com/]. > >> 3. firmware objects are persistent and read at boot time by interaction with >> firmware, unlike kernel objects which are not persistent. > That doesn't matter, sysfs exports what the hardware provides, and that > might persist over boot. > > So I don't see why a new filesystem is needed. > > You didn't explain why sysfs, or securitfs (except for the location in > the tree) does not work at all for your needs. The location really > doesn't matter all that much as you are creating a brand new location > anyway so we can just declare "this is where this stuff goes" and be ok. For rest of the questions, here is the summarized response. Based on mailing list previous discussions [1][2][3] and considering various firmware security use cases, our fwsecurityfs proposal seemed to be a reasonable and acceptable approach based on the feedback [4]. [1] https://lore.kernel.org/linuxppc-dev/YeuyUVVdFADCuDr4@kroah.com/#t [2] https://lore.kernel.org/linuxppc-dev/Yfk6gucNmJuR%2Fegi@kroah.com/ [3] https://lore.kernel.org/all/Yfo%2F5gYgb9Sv24YB@kroah.com/t/#m40250fdb3fddaafe502ab06e329e63381b00582d [4] https://lore.kernel.org/linuxppc-dev/YrQqPhi4+jHZ1WJc@kroah.com/ RFC v1 was using sysfs. After considering feedback[1][2][3], the following are design considerations for unification via fwsecurityfs: 1. Unify the location: Defining a security directory under /sys/firmware facilitates exposing objects related to firmware security features in a single place. Different platforms can create their respective directory structures within /sys/firmware/security. 2. Unify the code: To support unification, having the fwsecurityfs filesystem API allows different platforms to define the inode and file operations they need. fwsecurityfs provides a common API that can be used by each platform-specific implementation to support its particular requirements and interaction with firmware. Initializing platform-specific functions is the purpose of the fwsecurityfs_arch_init() function that is called on mount. Patch 3/4 implements fwsecurityfs_arch_init() for powerpc. Similar to the common place securityfs provides for LSMs to interact with kernel security objects, fwsecurityfs would provide a common place for all firmware security objects, which interact with the firmware rather than the kernel. Although at the API level, the two filesystem look similar, the requirements for firmware and kernel objects are different. Therefore, reusing securityfs wasn't a good fit for the firmware use case and we are proposing a similar but different filesystem - fwsecurityfs - focused for firmware security. > > And again, how are you going to get all Linux distros to now mount your > new filesystem? It would be analogous to the way securityfs is mounted. Thanks & Regards, - Nayna > > thanks, > > greg k-h
On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: > > On 11/10/22 04:58, Greg Kroah-Hartman wrote: > > On Wed, Nov 09, 2022 at 03:10:37PM -0500, Nayna wrote: > > > On 11/9/22 08:46, Greg Kroah-Hartman wrote: > > > > On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: > > > > > securityfs is meant for Linux security subsystems to expose policies/logs > > > > > or any other information. However, there are various firmware security > > > > > features which expose their variables for user management via the kernel. > > > > > There is currently no single place to expose these variables. Different > > > > > platforms use sysfs/platform specific filesystem(efivarfs)/securityfs > > > > > interface as they find it appropriate. Thus, there is a gap in kernel > > > > > interfaces to expose variables for security features. > > > > > > > > > > Define a firmware security filesystem (fwsecurityfs) to be used by > > > > > security features enabled by the firmware. These variables are platform > > > > > specific. This filesystem provides platforms a way to implement their > > > > > own underlying semantics by defining own inode and file operations. > > > > > > > > > > Similar to securityfs, the firmware security filesystem is recommended > > > > > to be exposed on a well known mount point /sys/firmware/security. > > > > > Platforms can define their own directory or file structure under this path. > > > > > > > > > > Example: > > > > > > > > > > # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security > > > > Why not juset use securityfs in /sys/security/firmware/ instead? Then > > > > you don't have to create a new filesystem and convince userspace to > > > > mount it in a specific location? > > > From man 5 sysfs page: > > > > > > /sys/firmware: This subdirectory contains interfaces for viewing and > > > manipulating firmware-specific objects and attributes. > > > > > > /sys/kernel: This subdirectory contains various files and subdirectories > > > that provide information about the running kernel. > > > > > > The security variables which are being exposed via fwsecurityfs are managed > > > by firmware, stored in firmware managed space and also often consumed by > > > firmware for enabling various security features. > > Ok, then just use the normal sysfs interface for /sys/firmware, why do > > you need a whole new filesystem type? > > > > > From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of > > > securityfs(/sys/kernel/security) is to provide a common place for all kernel > > > LSMs. The idea of > > > fwsecurityfs(/sys/firmware/security) is to similarly provide a common place > > > for all firmware security objects. > > > > > > /sys/firmware already exists. The patch now defines a new /security > > > directory in it for firmware security features. Using /sys/kernel/security > > > would mean scattering firmware objects in multiple places and confusing the > > > purpose of /sys/kernel and /sys/firmware. > > sysfs is confusing already, no problem with making it more confusing :) > > > > Just document where you add things and all should be fine. > > > > > Even though fwsecurityfs code is based on securityfs, since the two > > > filesystems expose different types of objects and have different > > > requirements, there are distinctions: > > > > > > 1. fwsecurityfs lets users create files in userspace, securityfs only allows > > > kernel subsystems to create files. > > Wait, why would a user ever create a file in this filesystem? If you > > need that, why not use configfs? That's what that is for, right? > > The purpose of fwsecurityfs is not to expose configuration items but rather > security objects used for firmware security features. I think these are more > comparable to EFI variables, which are exposed via an EFI-specific > filesystem, efivarfs, rather than configfs. > > > > > > 2. firmware and kernel objects may have different requirements. For example, > > > consideration of namespacing. As per my understanding, namespacing is > > > applied to kernel resources and not firmware resources. That's why it makes > > > sense to add support for namespacing in securityfs, but we concluded that > > > fwsecurityfs currently doesn't need it. Another but similar example of it > > > is: TPM space, which is exposed from hardware. For containers, the TPM would > > > be made as virtual/software TPM. Similarly for firmware space for > > > containers, it would have to be something virtualized/software version of > > > it. > > I do not understand, sorry. What does namespaces have to do with this? > > sysfs can already handle namespaces just fine, why not use that? > > Firmware objects are not namespaced. I mentioned it here as an example of > the difference between firmware and kernel objects. It is also in response > to the feedback from James Bottomley in RFC v2 [https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38adb59a66dcae4c59b.camel@HansenPartnership.com/]. I do not understand, sorry. Do you want to use a namespace for these or not? The code does not seem to be using namespaces. You can use sysfs with, or without, a namespace so I don't understand the issue here. With your code, there is no namespace. > > > 3. firmware objects are persistent and read at boot time by interaction with > > > firmware, unlike kernel objects which are not persistent. > > That doesn't matter, sysfs exports what the hardware provides, and that > > might persist over boot. > > > > So I don't see why a new filesystem is needed. > > > > You didn't explain why sysfs, or securitfs (except for the location in > > the tree) does not work at all for your needs. The location really > > doesn't matter all that much as you are creating a brand new location > > anyway so we can just declare "this is where this stuff goes" and be ok. > > For rest of the questions, here is the summarized response. > > Based on mailing list previous discussions [1][2][3] and considering various > firmware security use cases, our fwsecurityfs proposal seemed to be a > reasonable and acceptable approach based on the feedback [4]. > > [1] https://lore.kernel.org/linuxppc-dev/YeuyUVVdFADCuDr4@kroah.com/#t > [2] https://lore.kernel.org/linuxppc-dev/Yfk6gucNmJuR%2Fegi@kroah.com/ > [3] https://lore.kernel.org/all/Yfo%2F5gYgb9Sv24YB@kroah.com/t/#m40250fdb3fddaafe502ab06e329e63381b00582d > [4] https://lore.kernel.org/linuxppc-dev/YrQqPhi4+jHZ1WJc@kroah.com/ > > RFC v1 was using sysfs. After considering feedback[1][2][3], the following > are design considerations for unification via fwsecurityfs: > > 1. Unify the location: Defining a security directory under /sys/firmware > facilitates exposing objects related to firmware security features in a > single place. Different platforms can create their respective directory > structures within /sys/firmware/security. So just pick one place in sysfs for this to always go into. Your patch series does not document anything here, there are no Documentation/ABI/ entries that define the files being created, so that it's really hard to be able to review the code to determine if it is doing what you are wanting it to do. You can't document apis with just a changelog text alone, sorry. > 2. Unify the code: To support unification, having the fwsecurityfs > filesystem API allows different platforms to define the inode and file > operations they need. fwsecurityfs provides a common API that can be used by > each platform-specific implementation to support its particular requirements > and interaction with firmware. Initializing platform-specific functions is > the purpose of the fwsecurityfs_arch_init() function that is called on > mount. Patch 3/4 implements fwsecurityfs_arch_init() for powerpc. But you only are doing this for one platform, that's not any unification. APIs don't really work unless they can handle 3 users, as then you really understand if they work or not. Right now you wrote this code and it only has one user, that's a platform-specific-filesystem-only so far. > Similar to the common place securityfs provides for LSMs to interact with > kernel security objects, fwsecurityfs would provide a common place for all > firmware security objects, which interact with the firmware rather than the > kernel. Although at the API level, the two filesystem look similar, the > requirements for firmware and kernel objects are different. Therefore, > reusing securityfs wasn't a good fit for the firmware use case and we are > proposing a similar but different filesystem - fwsecurityfs - focused for > firmware security. What other platforms will use this? Who is going to move their code over to it? > > And again, how are you going to get all Linux distros to now mount your > > new filesystem? > > It would be analogous to the way securityfs is mounted. That did not answer the question. The question is how are you going to get the distros to mount your new filesystem specifically? How will they know that they need to modify their init scripts to do this? Who is going to do that? For what distro? On what timeline? Oh, and it looks like this series doesn't pass the kernel testing bot at all, so I'll not review the code until that's all fixed up at the very least. thanks, greg k-h
On 11/17/22 16:27, Greg Kroah-Hartman wrote: > On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: >> On 11/10/22 04:58, Greg Kroah-Hartman wrote: >>> On Wed, Nov 09, 2022 at 03:10:37PM -0500, Nayna wrote: >>>> On 11/9/22 08:46, Greg Kroah-Hartman wrote: >>>>> On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: >>>>>> securityfs is meant for Linux security subsystems to expose policies/logs >>>>>> or any other information. However, there are various firmware security >>>>>> features which expose their variables for user management via the kernel. >>>>>> There is currently no single place to expose these variables. Different >>>>>> platforms use sysfs/platform specific filesystem(efivarfs)/securityfs >>>>>> interface as they find it appropriate. Thus, there is a gap in kernel >>>>>> interfaces to expose variables for security features. >>>>>> >>>>>> Define a firmware security filesystem (fwsecurityfs) to be used by >>>>>> security features enabled by the firmware. These variables are platform >>>>>> specific. This filesystem provides platforms a way to implement their >>>>>> own underlying semantics by defining own inode and file operations. >>>>>> >>>>>> Similar to securityfs, the firmware security filesystem is recommended >>>>>> to be exposed on a well known mount point /sys/firmware/security. >>>>>> Platforms can define their own directory or file structure under this path. >>>>>> >>>>>> Example: >>>>>> >>>>>> # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security >>>>> Why not juset use securityfs in /sys/security/firmware/ instead? Then >>>>> you don't have to create a new filesystem and convince userspace to >>>>> mount it in a specific location? >>>> From man 5 sysfs page: >>>> >>>> /sys/firmware: This subdirectory contains interfaces for viewing and >>>> manipulating firmware-specific objects and attributes. >>>> >>>> /sys/kernel: This subdirectory contains various files and subdirectories >>>> that provide information about the running kernel. >>>> >>>> The security variables which are being exposed via fwsecurityfs are managed >>>> by firmware, stored in firmware managed space and also often consumed by >>>> firmware for enabling various security features. >>> Ok, then just use the normal sysfs interface for /sys/firmware, why do >>> you need a whole new filesystem type? >>> >>>> From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of >>>> securityfs(/sys/kernel/security) is to provide a common place for all kernel >>>> LSMs. The idea of >>>> fwsecurityfs(/sys/firmware/security) is to similarly provide a common place >>>> for all firmware security objects. >>>> >>>> /sys/firmware already exists. The patch now defines a new /security >>>> directory in it for firmware security features. Using /sys/kernel/security >>>> would mean scattering firmware objects in multiple places and confusing the >>>> purpose of /sys/kernel and /sys/firmware. >>> sysfs is confusing already, no problem with making it more confusing :) >>> >>> Just document where you add things and all should be fine. >>> >>>> Even though fwsecurityfs code is based on securityfs, since the two >>>> filesystems expose different types of objects and have different >>>> requirements, there are distinctions: >>>> >>>> 1. fwsecurityfs lets users create files in userspace, securityfs only allows >>>> kernel subsystems to create files. >>> Wait, why would a user ever create a file in this filesystem? If you >>> need that, why not use configfs? That's what that is for, right? >> The purpose of fwsecurityfs is not to expose configuration items but rather >> security objects used for firmware security features. I think these are more >> comparable to EFI variables, which are exposed via an EFI-specific >> filesystem, efivarfs, rather than configfs. >> >>>> 2. firmware and kernel objects may have different requirements. For example, >>>> consideration of namespacing. As per my understanding, namespacing is >>>> applied to kernel resources and not firmware resources. That's why it makes >>>> sense to add support for namespacing in securityfs, but we concluded that >>>> fwsecurityfs currently doesn't need it. Another but similar example of it >>>> is: TPM space, which is exposed from hardware. For containers, the TPM would >>>> be made as virtual/software TPM. Similarly for firmware space for >>>> containers, it would have to be something virtualized/software version of >>>> it. >>> I do not understand, sorry. What does namespaces have to do with this? >>> sysfs can already handle namespaces just fine, why not use that? >> Firmware objects are not namespaced. I mentioned it here as an example of >> the difference between firmware and kernel objects. It is also in response >> to the feedback from James Bottomley in RFC v2 [https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38adb59a66dcae4c59b.camel@HansenPartnership.com/]. > I do not understand, sorry. Do you want to use a namespace for these or > not? The code does not seem to be using namespaces. You can use sysfs > with, or without, a namespace so I don't understand the issue here. > > With your code, there is no namespace. You are correct. There's no namespace for these. > >>>> 3. firmware objects are persistent and read at boot time by interaction with >>>> firmware, unlike kernel objects which are not persistent. >>> That doesn't matter, sysfs exports what the hardware provides, and that >>> might persist over boot. >>> >>> So I don't see why a new filesystem is needed. >>> >>> You didn't explain why sysfs, or securitfs (except for the location in >>> the tree) does not work at all for your needs. The location really >>> doesn't matter all that much as you are creating a brand new location >>> anyway so we can just declare "this is where this stuff goes" and be ok. >> For rest of the questions, here is the summarized response. >> >> Based on mailing list previous discussions [1][2][3] and considering various >> firmware security use cases, our fwsecurityfs proposal seemed to be a >> reasonable and acceptable approach based on the feedback [4]. >> >> [1] https://lore.kernel.org/linuxppc-dev/YeuyUVVdFADCuDr4@kroah.com/#t >> [2] https://lore.kernel.org/linuxppc-dev/Yfk6gucNmJuR%2Fegi@kroah.com/ >> [3] https://lore.kernel.org/all/Yfo%2F5gYgb9Sv24YB@kroah.com/t/#m40250fdb3fddaafe502ab06e329e63381b00582d >> [4] https://lore.kernel.org/linuxppc-dev/YrQqPhi4+jHZ1WJc@kroah.com/ >> >> RFC v1 was using sysfs. After considering feedback[1][2][3], the following >> are design considerations for unification via fwsecurityfs: >> >> 1. Unify the location: Defining a security directory under /sys/firmware >> facilitates exposing objects related to firmware security features in a >> single place. Different platforms can create their respective directory >> structures within /sys/firmware/security. > So just pick one place in sysfs for this to always go into. I agree that the objects should go directly under a /sys/firmware/security mountpoint. > Your patch series does not document anything here, there are no > Documentation/ABI/ entries that define the files being created, so that > it's really hard to be able to review the code to determine if it is > doing what you are wanting it to do. > > You can't document apis with just a changelog text alone, sorry. Agreed, I'll include documentation in the next version. > >> 2. Unify the code: To support unification, having the fwsecurityfs >> filesystem API allows different platforms to define the inode and file >> operations they need. fwsecurityfs provides a common API that can be used by >> each platform-specific implementation to support its particular requirements >> and interaction with firmware. Initializing platform-specific functions is >> the purpose of the fwsecurityfs_arch_init() function that is called on >> mount. Patch 3/4 implements fwsecurityfs_arch_init() for powerpc. > But you only are doing this for one platform, that's not any > unification. APIs don't really work unless they can handle 3 users, as > then you really understand if they work or not. > > Right now you wrote this code and it only has one user, that's a > platform-specific-filesystem-only so far. Yes I agree, having more exploiters would certainly help to confirm and improve the interface. If you prefer, we could start with an arch specific filesystem. It could be made generic in the future if required. > >> Similar to the common place securityfs provides for LSMs to interact with >> kernel security objects, fwsecurityfs would provide a common place for all >> firmware security objects, which interact with the firmware rather than the >> kernel. Although at the API level, the two filesystem look similar, the >> requirements for firmware and kernel objects are different. Therefore, >> reusing securityfs wasn't a good fit for the firmware use case and we are >> proposing a similar but different filesystem - fwsecurityfs - focused for >> firmware security. > What other platforms will use this? Who is going to move their code > over to it? I had received constructive feedback on my RFC v2 but thus far, no other platforms have indicated they have a need for it. >>> And again, how are you going to get all Linux distros to now mount your >>> new filesystem? >> It would be analogous to the way securityfs is mounted. > That did not answer the question. The question is how are you going to > get the distros to mount your new filesystem specifically? How will > they know that they need to modify their init scripts to do this? Who > is going to do that? For what distro? On what timeline? I'll add a documentation patch for fwsecurityfs. And I'll propose a systemd patch to extend mount_table[] in src/shared/mount-setup.c to include fwsecurityfs. For RHEL 9.3 and SLES 15 SP6, we have feature requests opened to request adoption of the PKS userspace interface. We will communicate the mount point and init script changes via those feature requests. Other distros can adapt the upstream implementation to fit their requirements, such as mounting via simple init scripts without systemd for more constrained systems, using the systemd as an example. Please let me know if you have other concerns with respect to mounting the filesystem. > Oh, and it looks like this series doesn't pass the kernel testing bot at > all, so I'll not review the code until that's all fixed up at the very > least. I knew it failed, but I wanted to get your feedback on the approach before posting a new version. I'll fix it. Thank you for your review and feedback. I hope I have addressed your concerns. Thanks & Regards, - Nayna > thanks, > > greg k-h
Hello Nayna, On 22/11/09 03:10PM, Nayna wrote: > > On 11/9/22 08:46, Greg Kroah-Hartman wrote: > > On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: > > > securityfs is meant for Linux security subsystems to expose policies/logs > > > or any other information. However, there are various firmware security > > > features which expose their variables for user management via the kernel. > > > There is currently no single place to expose these variables. Different > > > platforms use sysfs/platform specific filesystem(efivarfs)/securityfs > > > interface as they find it appropriate. Thus, there is a gap in kernel > > > interfaces to expose variables for security features. > > > > > > Define a firmware security filesystem (fwsecurityfs) to be used by > > > security features enabled by the firmware. These variables are platform > > > specific. This filesystem provides platforms a way to implement their > > > own underlying semantics by defining own inode and file operations. > > > > > > Similar to securityfs, the firmware security filesystem is recommended > > > to be exposed on a well known mount point /sys/firmware/security. > > > Platforms can define their own directory or file structure under this path. > > > > > > Example: > > > > > > # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security > > Why not juset use securityfs in /sys/security/firmware/ instead? Then > > you don't have to create a new filesystem and convince userspace to > > mount it in a specific location? I am also curious to know on why not use securityfs, given the similarity between the two. :) More specifics on that below... > > From man 5 sysfs page: > > /sys/firmware: This subdirectory contains interfaces for viewing and > manipulating firmware-specific objects and attributes. > > /sys/kernel: This subdirectory contains various files and subdirectories > that provide information about the running kernel. > > The security variables which are being exposed via fwsecurityfs are managed > by firmware, stored in firmware managed space and also often consumed by > firmware for enabling various security features. That's ok. As I see it users of securityfs can define their own fileops (like how you are doing in fwsecurityfs). See securityfs_create_file() & securityfs_create_symlink(), can accept the fops & iops. Except maybe securityfs_create_dir(), that could be since there might not be a usecase for it. But do you also need it in your case is the question to ask. > > From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of > securityfs(/sys/kernel/security) is to provide a common place for all kernel > LSMs. The idea of Which was then seperated out by commit, da31894ed7b654e2 ("securityfs: do not depend on CONFIG_SECURITY"). securityfs now has a seperate CONFIG_SECURITYFS config option. In fact I was even thinking of why shouldn't we move security/inode.c into fs/securityfs/inode.c . fs/* is a common place for all filesystems. Users of securityfs can call it's exported kernel APIs to create files/dirs/symlinks. If we move security/inode.c to fs/security/inode.c, then... ...below call within securityfs_init() should be moved into some lsm sepecific file. #ifdef CONFIG_SECURITY static struct dentry *lsm_dentry; static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count, loff_t *ppos) { return simple_read_from_buffer(buf, count, ppos, lsm_names, strlen(lsm_names)); } static const struct file_operations lsm_ops = { .read = lsm_read, .llseek = generic_file_llseek, }; #endif securityfs_init() #ifdef CONFIG_SECURITY lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL, &lsm_ops); #endif So why not move it? Maybe others, can comment more on whether it's a good idea to move security/inode.c into fs/security/inode.c? This should then help others identify securityfs filesystem in fs/security/ for everyone to notice and utilize for their use? > fwsecurityfs(/sys/firmware/security) is to similarly provide a common place > for all firmware security objects. > > /sys/firmware already exists. The patch now defines a new /security > directory in it for firmware security features. Using /sys/kernel/security > would mean scattering firmware objects in multiple places and confusing the > purpose of /sys/kernel and /sys/firmware. We can also think of it this way that, all security related exports should happen via /sys/kernel/security/. Then /sys/kernel/security/firmware/ becomes the security related firmware exports. If you see find /sys -iname firmware, I am sure you will find other firmware specifics directories related to other specific subsystems (e.g. root@qemu:/home/qemu# find /sys -iname firmware /sys/devices/ndbus0/nmem0/firmware /sys/devices/ndbus0/firmware /sys/firmware ) But it could be, I am not an expert here, although I was thinking a good Documentation might solve this problem. > > Even though fwsecurityfs code is based on securityfs, since the two > filesystems expose different types of objects and have different > requirements, there are distinctions: > > 1. fwsecurityfs lets users create files in userspace, securityfs only allows > kernel subsystems to create files. Sorry could you please elaborate how? both securityfs & fwsecurityfs calls simple_fill_super() which uses the same inode (i_op) and inode file operations (i_fop) from fs/libfs.c for their root inode. So how it is enabling user (as in userspace) to create a file in this filesystem? So am I missing anything? > > 2. firmware and kernel objects may have different requirements. For example, > consideration of namespacing. As per my understanding, namespacing is > applied to kernel resources and not firmware resources. That's why it makes > sense to add support for namespacing in securityfs, but we concluded that > fwsecurityfs currently doesn't need it. Another but similar example of it It "currently" doesn't need it. But can it in future? Then why not go with securityfs which has an additional namespacing feature available? That's actually also the point of utilizing an existing FS which can get features like this in future. As long as it doesn't affect the functionality of your use case, we simply need not reject securityfs, no? > is: TPM space, which is exposed from hardware. For containers, the TPM would > be made as virtual/software TPM. Similarly for firmware space for > containers, it would have to be something virtualized/software version of > it. > > 3. firmware objects are persistent and read at boot time by interaction with > firmware, unlike kernel objects which are not persistent. I think this got addressed in a seperate thread. -ritesh
On Sat, Nov 19, 2022 at 01:20:09AM -0500, Nayna wrote: > > On 11/17/22 16:27, Greg Kroah-Hartman wrote: > > On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: > > > On 11/10/22 04:58, Greg Kroah-Hartman wrote: > > > > On Wed, Nov 09, 2022 at 03:10:37PM -0500, Nayna wrote: > > > > > On 11/9/22 08:46, Greg Kroah-Hartman wrote: > > > > > > On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: > > > > > > > securityfs is meant for Linux security subsystems to expose policies/logs > > > > > > > or any other information. However, there are various firmware security > > > > > > > features which expose their variables for user management via the kernel. > > > > > > > There is currently no single place to expose these variables. Different > > > > > > > platforms use sysfs/platform specific filesystem(efivarfs)/securityfs > > > > > > > interface as they find it appropriate. Thus, there is a gap in kernel > > > > > > > interfaces to expose variables for security features. > > > > > > > > > > > > > > Define a firmware security filesystem (fwsecurityfs) to be used by > > > > > > > security features enabled by the firmware. These variables are platform > > > > > > > specific. This filesystem provides platforms a way to implement their > > > > > > > own underlying semantics by defining own inode and file operations. > > > > > > > > > > > > > > Similar to securityfs, the firmware security filesystem is recommended > > > > > > > to be exposed on a well known mount point /sys/firmware/security. > > > > > > > Platforms can define their own directory or file structure under this path. > > > > > > > > > > > > > > Example: > > > > > > > > > > > > > > # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security > > > > > > Why not juset use securityfs in /sys/security/firmware/ instead? Then > > > > > > you don't have to create a new filesystem and convince userspace to > > > > > > mount it in a specific location? > > > > > From man 5 sysfs page: > > > > > > > > > > /sys/firmware: This subdirectory contains interfaces for viewing and > > > > > manipulating firmware-specific objects and attributes. > > > > > > > > > > /sys/kernel: This subdirectory contains various files and subdirectories > > > > > that provide information about the running kernel. > > > > > > > > > > The security variables which are being exposed via fwsecurityfs are managed > > > > > by firmware, stored in firmware managed space and also often consumed by > > > > > firmware for enabling various security features. > > > > Ok, then just use the normal sysfs interface for /sys/firmware, why do > > > > you need a whole new filesystem type? > > > > > > > > > From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of > > > > > securityfs(/sys/kernel/security) is to provide a common place for all kernel > > > > > LSMs. The idea of > > > > > fwsecurityfs(/sys/firmware/security) is to similarly provide a common place > > > > > for all firmware security objects. > > > > > > > > > > /sys/firmware already exists. The patch now defines a new /security > > > > > directory in it for firmware security features. Using /sys/kernel/security > > > > > would mean scattering firmware objects in multiple places and confusing the > > > > > purpose of /sys/kernel and /sys/firmware. > > > > sysfs is confusing already, no problem with making it more confusing :) > > > > > > > > Just document where you add things and all should be fine. > > > > > > > > > Even though fwsecurityfs code is based on securityfs, since the two > > > > > filesystems expose different types of objects and have different > > > > > requirements, there are distinctions: > > > > > > > > > > 1. fwsecurityfs lets users create files in userspace, securityfs only allows > > > > > kernel subsystems to create files. > > > > Wait, why would a user ever create a file in this filesystem? If you > > > > need that, why not use configfs? That's what that is for, right? > > > The purpose of fwsecurityfs is not to expose configuration items but rather > > > security objects used for firmware security features. I think these are more > > > comparable to EFI variables, which are exposed via an EFI-specific > > > filesystem, efivarfs, rather than configfs. > > > > > > > > 2. firmware and kernel objects may have different requirements. For example, > > > > > consideration of namespacing. As per my understanding, namespacing is > > > > > applied to kernel resources and not firmware resources. That's why it makes > > > > > sense to add support for namespacing in securityfs, but we concluded that > > > > > fwsecurityfs currently doesn't need it. Another but similar example of it > > > > > is: TPM space, which is exposed from hardware. For containers, the TPM would > > > > > be made as virtual/software TPM. Similarly for firmware space for > > > > > containers, it would have to be something virtualized/software version of > > > > > it. > > > > I do not understand, sorry. What does namespaces have to do with this? > > > > sysfs can already handle namespaces just fine, why not use that? > > > Firmware objects are not namespaced. I mentioned it here as an example of > > > the difference between firmware and kernel objects. It is also in response > > > to the feedback from James Bottomley in RFC v2 [https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38adb59a66dcae4c59b.camel@HansenPartnership.com/]. > > I do not understand, sorry. Do you want to use a namespace for these or > > not? The code does not seem to be using namespaces. You can use sysfs > > with, or without, a namespace so I don't understand the issue here. > > > > With your code, there is no namespace. > > You are correct. There's no namespace for these. So again, I do not understand. Do you want to use filesystem namespaces, or do you not? How again can you not use sysfs or securityfs due to namespaces? What is missing? confused, greg k-h
On Sun, 2022-11-20 at 17:13 +0100, Greg Kroah-Hartman wrote: > On Sat, Nov 19, 2022 at 01:20:09AM -0500, Nayna wrote: > > > > On 11/17/22 16:27, Greg Kroah-Hartman wrote: > > > On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: > > > > On 11/10/22 04:58, Greg Kroah-Hartman wrote: [...] > > > > > I do not understand, sorry. What does namespaces have to do > > > > > with this? > > > > > sysfs can already handle namespaces just fine, why not use > > > > > that? > > > > Firmware objects are not namespaced. I mentioned it here as an > > > > example of the difference between firmware and kernel objects. > > > > It is also in response to the feedback from James Bottomley in > > > > RFC v2 [ > > > > https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38ad > > > > b59a66dcae4c59b.camel@HansenPartnership.com/]. > > > I do not understand, sorry. Do you want to use a namespace for > > > these or not? The code does not seem to be using namespaces. > > > You can use sysfs with, or without, a namespace so I don't > > > understand the issue here. > > > > > > With your code, there is no namespace. > > > > You are correct. There's no namespace for these. > > So again, I do not understand. Do you want to use filesystem > namespaces, or do you not? Since this seems to go back to my email quoted again, let me repeat: the question isn't if this patch is namespaced; I think you've agreed several times it isn't. The question is if the exposed properties would ever need to be namespaced. This is a subtle and complex question which isn't at all explored by the above interchange. > How again can you not use sysfs or securityfs due to namespaces? > What is missing? I already explained in the email that sysfs contains APIs like simple_pin_... which are completely inimical to namespacing. Currently securityfs contains them as well, so in that regard they're both no better than each other. The point I was making is that securityfs is getting namespaced by the IMA namespace rework (which is pretty complex due to having to replace the simple_pin_... APIs), so when (perhaps if) the IMA namespace is accepted, securityfs will make a good home for quantities that need namespacing. That's not to say you can't namespace things in sysfs, you can, in the same way that you can get a round peg into a square hole if you bang hard enough. So perhaps we could get back to the original question of whether these quantities would ever be namespaced ... or, conversely, whether they would never need namespacing. James
On Sun, Nov 20, 2022 at 10:14:26PM -0500, James Bottomley wrote: > On Sun, 2022-11-20 at 17:13 +0100, Greg Kroah-Hartman wrote: > > On Sat, Nov 19, 2022 at 01:20:09AM -0500, Nayna wrote: > > > > > > On 11/17/22 16:27, Greg Kroah-Hartman wrote: > > > > On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: > > > > > On 11/10/22 04:58, Greg Kroah-Hartman wrote: > [...] > > > > > > I do not understand, sorry. What does namespaces have to do > > > > > > with this? > > > > > > sysfs can already handle namespaces just fine, why not use > > > > > > that? > > > > > Firmware objects are not namespaced. I mentioned it here as an > > > > > example of the difference between firmware and kernel objects. > > > > > It is also in response to the feedback from James Bottomley in > > > > > RFC v2 [ > > > > > https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38ad > > > > > b59a66dcae4c59b.camel@HansenPartnership.com/]. > > > > I do not understand, sorry. Do you want to use a namespace for > > > > these or not? The code does not seem to be using namespaces. > > > > You can use sysfs with, or without, a namespace so I don't > > > > understand the issue here. > > > > > > > > With your code, there is no namespace. > > > > > > You are correct. There's no namespace for these. > > > > So again, I do not understand. Do you want to use filesystem > > namespaces, or do you not? > > Since this seems to go back to my email quoted again, let me repeat: > the question isn't if this patch is namespaced; I think you've agreed > several times it isn't. The question is if the exposed properties > would ever need to be namespaced. This is a subtle and complex > question which isn't at all explored by the above interchange. > > > How again can you not use sysfs or securityfs due to namespaces? > > What is missing? > > I already explained in the email that sysfs contains APIs like > simple_pin_... which are completely inimical to namespacing. Then how does the networking code handle the namespace stuff in sysfs? That seems to work today, or am I missing something? If the namespace support needs to be fixed up in sysfs (or in securityfs), then great, let's do that, and not write a whole new filesystem just because that's not done. Also this patch series also doesn't handle namespaces, so again, I am totally confused as to why this is even being discussed... thanks, greg k-h
On Mon, 2022-11-21 at 12:05 +0100, Greg Kroah-Hartman wrote: > On Sun, Nov 20, 2022 at 10:14:26PM -0500, James Bottomley wrote: > > On Sun, 2022-11-20 at 17:13 +0100, Greg Kroah-Hartman wrote: > > > On Sat, Nov 19, 2022 at 01:20:09AM -0500, Nayna wrote: > > > > > > > > On 11/17/22 16:27, Greg Kroah-Hartman wrote: > > > > > On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: > > > > > > On 11/10/22 04:58, Greg Kroah-Hartman wrote: > > [...] > > > > > > > I do not understand, sorry. What does namespaces have to > > > > > > > do > > > > > > > with this? > > > > > > > sysfs can already handle namespaces just fine, why not > > > > > > > use > > > > > > > that? > > > > > > Firmware objects are not namespaced. I mentioned it here as > > > > > > an > > > > > > example of the difference between firmware and kernel > > > > > > objects. > > > > > > It is also in response to the feedback from James Bottomley > > > > > > in > > > > > > RFC v2 [ > > > > > > https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38ad > > > > > > b59a66dcae4c59b.camel@HansenPartnership.com/]. > > > > > I do not understand, sorry. Do you want to use a namespace > > > > > for > > > > > these or not? The code does not seem to be using > > > > > namespaces. > > > > > You can use sysfs with, or without, a namespace so I don't > > > > > understand the issue here. > > > > > > > > > > With your code, there is no namespace. > > > > > > > > You are correct. There's no namespace for these. > > > > > > So again, I do not understand. Do you want to use filesystem > > > namespaces, or do you not? > > > > Since this seems to go back to my email quoted again, let me > > repeat: the question isn't if this patch is namespaced; I think > > you've agreed several times it isn't. The question is if the > > exposed properties would ever need to be namespaced. This is a > > subtle and complex question which isn't at all explored by the > > above interchange. > > > > > How again can you not use sysfs or securityfs due to namespaces? > > > What is missing? > > > > I already explained in the email that sysfs contains APIs like > > simple_pin_... which are completely inimical to namespacing. > > Then how does the networking code handle the namespace stuff in > sysfs? > That seems to work today, or am I missing something? have you actually tried? jejb@lingrow:~> sudo unshare --net bash lingrow:/home/jejb # ls /sys/class/net/ lo tun0 tun10 wlan0 lingrow:/home/jejb # ip link show 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group default qlen 1000 link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 So, as you see, I've entered a network namespace and ip link shows me the only interface I can see in that namespace (a down loopback) but sysfs shows me every interface on the system outside the namespace. This is pretty much the story of containers and sysfs: if you mount it inside the container, it leaks information about the host configuration. Since I created a container with full root, I could actually fiddle with the host network parameters on interfaces I shouldn't be able to see within the container using sysfs ... which is one reason we try to persuade people to use a user namespace instead of full root. > If the namespace support needs to be fixed up in sysfs (or in > securityfs), then great, let's do that, and not write a whole new > filesystem just because that's not done. As I said: a fix is proposed for securityfs. I think everyone in containers concluded long ago that sysfs is too big an Augean Stable. > Also this patch series also doesn't handle namespaces, so again, I am > totally confused as to why this is even being discussed... Well, it's not my patch. I came into this saying *if* there was ever a reason to namespace these parameters then please don't use interfaces inimical to namespacing. My personal view is that this should all just go in securityfs because that defers answering the question of whether it would eventually be namespaced. James
On Mon, Nov 21, 2022 at 09:03:18AM -0500, James Bottomley wrote: > On Mon, 2022-11-21 at 12:05 +0100, Greg Kroah-Hartman wrote: > > On Sun, Nov 20, 2022 at 10:14:26PM -0500, James Bottomley wrote: > > > On Sun, 2022-11-20 at 17:13 +0100, Greg Kroah-Hartman wrote: > > > > On Sat, Nov 19, 2022 at 01:20:09AM -0500, Nayna wrote: > > > > > > > > > > On 11/17/22 16:27, Greg Kroah-Hartman wrote: > > > > > > On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: > > > > > > > On 11/10/22 04:58, Greg Kroah-Hartman wrote: > > > [...] > > > > > > > > I do not understand, sorry. What does namespaces have to > > > > > > > > do > > > > > > > > with this? > > > > > > > > sysfs can already handle namespaces just fine, why not > > > > > > > > use > > > > > > > > that? > > > > > > > Firmware objects are not namespaced. I mentioned it here as > > > > > > > an > > > > > > > example of the difference between firmware and kernel > > > > > > > objects. > > > > > > > It is also in response to the feedback from James Bottomley > > > > > > > in > > > > > > > RFC v2 [ > > > > > > > https://lore.kernel.org/linuxppc-dev/41ca51e8db9907d9060cc38ad > > > > > > > b59a66dcae4c59b.camel@HansenPartnership.com/]. > > > > > > I do not understand, sorry. Do you want to use a namespace > > > > > > for > > > > > > these or not? The code does not seem to be using > > > > > > namespaces. > > > > > > You can use sysfs with, or without, a namespace so I don't > > > > > > understand the issue here. > > > > > > > > > > > > With your code, there is no namespace. > > > > > > > > > > You are correct. There's no namespace for these. > > > > > > > > So again, I do not understand. Do you want to use filesystem > > > > namespaces, or do you not? > > > > > > Since this seems to go back to my email quoted again, let me > > > repeat: the question isn't if this patch is namespaced; I think > > > you've agreed several times it isn't. The question is if the > > > exposed properties would ever need to be namespaced. This is a > > > subtle and complex question which isn't at all explored by the > > > above interchange. > > > > > > > How again can you not use sysfs or securityfs due to namespaces? > > > > What is missing? > > > > > > I already explained in the email that sysfs contains APIs like > > > simple_pin_... which are completely inimical to namespacing. > > > > Then how does the networking code handle the namespace stuff in > > sysfs? > > That seems to work today, or am I missing something? > > have you actually tried? > > jejb@lingrow:~> sudo unshare --net bash > lingrow:/home/jejb # ls /sys/class/net/ > lo tun0 tun10 wlan0 > lingrow:/home/jejb # ip link show > 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group > default qlen 1000 > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > > So, as you see, I've entered a network namespace and ip link shows me > the only interface I can see in that namespace (a down loopback) but > sysfs shows me every interface on the system outside the namespace. Then all of the code in include/kobject_ns.h is not being used? We have a whole kobject namespace set up for networking, I just assumed they were using it. If not, I'm all for ripping it out. thanks, greg k-h
From: James Bottomley > Sent: 21 November 2022 14:03 ... > > Then how does the networking code handle the namespace stuff in > > sysfs? > > That seems to work today, or am I missing something? > > have you actually tried? > > jejb@lingrow:~> sudo unshare --net bash > lingrow:/home/jejb # ls /sys/class/net/ > lo tun0 tun10 wlan0 > lingrow:/home/jejb # ip link show > 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT group > default qlen 1000 > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > > So, as you see, I've entered a network namespace and ip link shows me > the only interface I can see in that namespace (a down loopback) but > sysfs shows me every interface on the system outside the namespace. You have to remount /sys to get the restricted copy. eg by running 'ip netns exec namespace command'. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, 2022-11-21 at 16:05 +0100, Greg Kroah-Hartman wrote: > On Mon, Nov 21, 2022 at 09:03:18AM -0500, James Bottomley wrote: > > On Mon, 2022-11-21 at 12:05 +0100, Greg Kroah-Hartman wrote: > > > On Sun, Nov 20, 2022 at 10:14:26PM -0500, James Bottomley wrote: [...] > > > > I already explained in the email that sysfs contains APIs like > > > > simple_pin_... which are completely inimical to namespacing. > > > > > > Then how does the networking code handle the namespace stuff in > > > sysfs? That seems to work today, or am I missing something? > > > > have you actually tried? > > > > jejb@lingrow:~> sudo unshare --net bash > > lingrow:/home/jejb # ls /sys/class/net/ > > lo tun0 tun10 wlan0 > > lingrow:/home/jejb # ip link show > > 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT > > group > > default qlen 1000 > > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > > > > So, as you see, I've entered a network namespace and ip link shows > > me the only interface I can see in that namespace (a down loopback) > > but sysfs shows me every interface on the system outside the > > namespace. > > Then all of the code in include/kobject_ns.h is not being used? We > have a whole kobject namespace set up for networking, I just assumed > they were using it. If not, I'm all for ripping it out. Hm, looking at the implementation, it seems to trigger off the superblock (meaning you have to remount inside a mount namespace) and it only works to control visibility in label based namespaces, so this does actually work jejb@lingrow:~/git/linux> sudo unshare --net --mount bash lingrow:/home/jejb # mount -t sysfs none /sys lingrow:/home/jejb # ls /sys/class/net/ lo The label based approach means that any given file can be shown in one and only one namespace, which works for net, but not much else (although it probably could be adapted). James
On Mon, Nov 21, 2022 at 12:33:55PM -0500, James Bottomley wrote: > On Mon, 2022-11-21 at 16:05 +0100, Greg Kroah-Hartman wrote: > > On Mon, Nov 21, 2022 at 09:03:18AM -0500, James Bottomley wrote: > > > On Mon, 2022-11-21 at 12:05 +0100, Greg Kroah-Hartman wrote: > > > > On Sun, Nov 20, 2022 at 10:14:26PM -0500, James Bottomley wrote: > [...] > > > > > I already explained in the email that sysfs contains APIs like > > > > > simple_pin_... which are completely inimical to namespacing. > > > > > > > > Then how does the networking code handle the namespace stuff in > > > > sysfs? That seems to work today, or am I missing something? > > > > > > have you actually tried? > > > > > > jejb@lingrow:~> sudo unshare --net bash > > > lingrow:/home/jejb # ls /sys/class/net/ > > > lo tun0 tun10 wlan0 > > > lingrow:/home/jejb # ip link show > > > 1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN mode DEFAULT > > > group > > > default qlen 1000 > > > link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 > > > > > > So, as you see, I've entered a network namespace and ip link shows > > > me the only interface I can see in that namespace (a down loopback) > > > but sysfs shows me every interface on the system outside the > > > namespace. > > > > Then all of the code in include/kobject_ns.h is not being used? We > > have a whole kobject namespace set up for networking, I just assumed > > they were using it. If not, I'm all for ripping it out. > > Hm, looking at the implementation, it seems to trigger off the > superblock (meaning you have to remount inside a mount namespace) and > it only works to control visibility in label based namespaces, so this > does actually work > > jejb@lingrow:~/git/linux> sudo unshare --net --mount bash > lingrow:/home/jejb # mount -t sysfs none /sys > lingrow:/home/jejb # ls /sys/class/net/ > lo > > The label based approach means that any given file can be shown in one > and only one namespace, which works for net, but not much else > (although it probably could be adapted). Great, thanks for verifying it works properly. No other subsystem other than networking has cared about adding support for namespaces to their sysfs representations. But the base logic is all there if they want to do so. thanks, greg k-h
On 11/20/22 22:14, James Bottomley wrote: > On Sun, 2022-11-20 at 17:13 +0100, Greg Kroah-Hartman wrote: >> On Sat, Nov 19, 2022 at 01:20:09AM -0500, Nayna wrote: >>> On 11/17/22 16:27, Greg Kroah-Hartman wrote: >>>> On Mon, Nov 14, 2022 at 06:03:43PM -0500, Nayna wrote: >>>>> On 11/10/22 04:58, Greg Kroah-Hartman wrote: > [...] >>> [...] >>> You are correct. There's no namespace for these. >> So again, I do not understand. Do you want to use filesystem >> namespaces, or do you not? > Since this seems to go back to my email quoted again, let me repeat: > the question isn't if this patch is namespaced; I think you've agreed > several times it isn't. The question is if the exposed properties > would ever need to be namespaced. This is a subtle and complex > question which isn't at all explored by the above interchange. > >> How again can you not use sysfs or securityfs due to namespaces? >> What is missing? > I already explained in the email that sysfs contains APIs like > simple_pin_... which are completely inimical to namespacing. Currently > securityfs contains them as well, so in that regard they're both no > better than each other. The point I was making is that securityfs is > getting namespaced by the IMA namespace rework (which is pretty complex > due to having to replace the simple_pin_... APIs), so when (perhaps if) > the IMA namespace is accepted, securityfs will make a good home for > quantities that need namespacing. That's not to say you can't > namespace things in sysfs, you can, in the same way that you can get a > round peg into a square hole if you bang hard enough. > > So perhaps we could get back to the original question of whether these > quantities would ever be namespaced ... or, conversely, whether they > would never need namespacing. To clarify, I brought up in the discussion about namespacing considerations because I was asked about them. However, I determined there were none because firmware object interactions are invariant across namespaces. I don't see this changing in the future given that the firmware objects have no notion of namespacing. Thanks & Regards, - Nayna
On 11/19/22 06:48, Ritesh Harjani (IBM) wrote: > Hello Nayna, Hi Ritesh, > > On 22/11/09 03:10PM, Nayna wrote: >> On 11/9/22 08:46, Greg Kroah-Hartman wrote: >>> On Sun, Nov 06, 2022 at 04:07:42PM -0500, Nayna Jain wrote: >>>> securityfs is meant for Linux security subsystems to expose policies/logs >>>> or any other information. However, there are various firmware security >>>> features which expose their variables for user management via the kernel. >>>> There is currently no single place to expose these variables. Different >>>> platforms use sysfs/platform specific filesystem(efivarfs)/securityfs >>>> interface as they find it appropriate. Thus, there is a gap in kernel >>>> interfaces to expose variables for security features. >>>> >>>> Define a firmware security filesystem (fwsecurityfs) to be used by >>>> security features enabled by the firmware. These variables are platform >>>> specific. This filesystem provides platforms a way to implement their >>>> own underlying semantics by defining own inode and file operations. >>>> >>>> Similar to securityfs, the firmware security filesystem is recommended >>>> to be exposed on a well known mount point /sys/firmware/security. >>>> Platforms can define their own directory or file structure under this path. >>>> >>>> Example: >>>> >>>> # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security >>> Why not juset use securityfs in /sys/security/firmware/ instead? Then >>> you don't have to create a new filesystem and convince userspace to >>> mount it in a specific location? > I am also curious to know on why not use securityfs, given the similarity > between the two. :) > More specifics on that below... > >> From man 5 sysfs page: >> >> /sys/firmware: This subdirectory contains interfaces for viewing and >> manipulating firmware-specific objects and attributes. >> >> /sys/kernel: This subdirectory contains various files and subdirectories >> that provide information about the running kernel. >> >> The security variables which are being exposed via fwsecurityfs are managed >> by firmware, stored in firmware managed space and also often consumed by >> firmware for enabling various security features. > That's ok. As I see it users of securityfs can define their own fileops > (like how you are doing in fwsecurityfs). > See securityfs_create_file() & securityfs_create_symlink(), can accept the fops > & iops. Except maybe securityfs_create_dir(), that could be since there might > not be a usecase for it. But do you also need it in your case is the question to > ask. Please refer to the function plpks_secvars_init() in Patch 4/4. > >> From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose of >> securityfs(/sys/kernel/security) is to provide a common place for all kernel >> LSMs. The idea of > Which was then seperated out by commit, > da31894ed7b654e2 ("securityfs: do not depend on CONFIG_SECURITY"). > > securityfs now has a seperate CONFIG_SECURITYFS config option. In fact I was even > thinking of why shouldn't we move security/inode.c into fs/securityfs/inode.c . > fs/* is a common place for all filesystems. Users of securityfs can call it's > exported kernel APIs to create files/dirs/symlinks. > > If we move security/inode.c to fs/security/inode.c, then... > ...below call within securityfs_init() should be moved into some lsm sepecific > file. > > #ifdef CONFIG_SECURITY > static struct dentry *lsm_dentry; > static ssize_t lsm_read(struct file *filp, char __user *buf, size_t count, > loff_t *ppos) > { > return simple_read_from_buffer(buf, count, ppos, lsm_names, > strlen(lsm_names)); > } > > static const struct file_operations lsm_ops = { > .read = lsm_read, > .llseek = generic_file_llseek, > }; > #endif > > securityfs_init() > > #ifdef CONFIG_SECURITY > lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL, > &lsm_ops); > #endif > > So why not move it? Maybe others, can comment more on whether it's a good idea > to move security/inode.c into fs/security/inode.c? > This should then help others identify securityfs filesystem in fs/security/ > for everyone to notice and utilize for their use? >> fwsecurityfs(/sys/firmware/security) is to similarly provide a common place >> for all firmware security objects. >> >> /sys/firmware already exists. The patch now defines a new /security >> directory in it for firmware security features. Using /sys/kernel/security >> would mean scattering firmware objects in multiple places and confusing the >> purpose of /sys/kernel and /sys/firmware. > We can also think of it this way that, all security related exports should > happen via /sys/kernel/security/. Then /sys/kernel/security/firmware/ becomes > the security related firmware exports. > > If you see find /sys -iname firmware, I am sure you will find other firmware > specifics directories related to other specific subsystems > (e.g. > root@qemu:/home/qemu# find /sys -iname firmware > /sys/devices/ndbus0/nmem0/firmware > /sys/devices/ndbus0/firmware > /sys/firmware > ) > > But it could be, I am not an expert here, although I was thinking a good > Documentation might solve this problem. Documentation on sysfs(https://man7.org/linux/man-pages/man5/sysfs.5.html) already differentiates /sys/firmware and /sys/kernel as I responded earlier. The objects we are exposing are firmware objects and not kernel objects. > >> Even though fwsecurityfs code is based on securityfs, since the two >> filesystems expose different types of objects and have different >> requirements, there are distinctions: >> >> 1. fwsecurityfs lets users create files in userspace, securityfs only allows >> kernel subsystems to create files. > Sorry could you please elaborate how? both securityfs & fwsecurityfs > calls simple_fill_super() which uses the same inode (i_op) and inode file > operations (i_fop) from fs/libfs.c for their root inode. So how it is enabling > user (as in userspace) to create a file in this filesystem? > > So am I missing anything? The ability to let user(as in userspace) to create a file in a filesystem comes by allowing to define inode operations. Please look at the implementation differences for functions xxx_create_dir() and xxx_create_dentry() of securityfs vs fwsecurityfs. Also refer to Patch 4/4 for use of fwsecurityfs_create_dir() where inode operations are defined. > >> 2. firmware and kernel objects may have different requirements. For example, >> consideration of namespacing. As per my understanding, namespacing is >> applied to kernel resources and not firmware resources. That's why it makes >> sense to add support for namespacing in securityfs, but we concluded that >> fwsecurityfs currently doesn't need it. Another but similar example of it > It "currently" doesn't need it. But can it in future? Then why not go with > securityfs which has an additional namespacing feature available? > That's actually also the point of utilizing an existing FS which can get > features like this in future. As long as it doesn't affect the functionality > of your use case, we simply need not reject securityfs, no? Thanks for your review and feedback. To summarize: From the perspective of our use case, we need to expose firmware security objects to userspace for management. Not all of the objects pre-exist and we would like to allow root to create them from userspace. From a unification perspective, I have considered a common location at /sys/firmware/security for managing any platform's security objects. And I've proposed a generic filesystem, which could be used by any platform to represent firmware security objects via /sys/firmware/security. Here are some alternatives to generic filesystem in discussion: 1. Start with a platform-specific filesystem. If more platforms would like to use the approach, it can be made generic. We would still have a common location of /sys/firmware/security and new code would live in arch. This is my preference and would be the best fit for our use case. 2. Use securityfs. This would mean modifying it to satisfy other use cases, including supporting userspace file creation. I don't know if the securityfs maintainer would find that acceptable. I would also still want some way to expose variables at /sys/firmware/security. 3. Use a sysfs-based approach. This would be a platform-specific implementation. However, sysfs has a similar issue to securityfs for file creation. When I tried it in RFC v1[1], I had to implement a workaround to achieve that. [1] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-nayna@linux.ibm.com/ Thanks & Regards, - Nayna
On 11/22/22 18:21, Nayna wrote: > > From the perspective of our use case, we need to expose firmware > security objects to userspace for management. Not all of the objects > pre-exist and we would like to allow root to create them from userspace. > > From a unification perspective, I have considered a common location at > /sys/firmware/security for managing any platform's security objects. > And I've proposed a generic filesystem, which could be used by any > platform to represent firmware security objects via > /sys/firmware/security. > > Here are some alternatives to generic filesystem in discussion: > > 1. Start with a platform-specific filesystem. If more platforms would > like to use the approach, it can be made generic. We would still have > a common location of /sys/firmware/security and new code would live in > arch. This is my preference and would be the best fit for our use case. > > 2. Use securityfs. This would mean modifying it to satisfy other use > cases, including supporting userspace file creation. I don't know if > the securityfs maintainer would find that acceptable. I would also > still want some way to expose variables at /sys/firmware/security. > > 3. Use a sysfs-based approach. This would be a platform-specific > implementation. However, sysfs has a similar issue to securityfs for > file creation. When I tried it in RFC v1[1], I had to implement a > workaround to achieve that. > > [1] > https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-nayna@linux.ibm.com/ > Hi Greg, Based on the discussions so far, is Option 1, described above, an acceptable next step? Thanks & Regards, - Nayna
On Wed, Nov 23, 2022 at 10:05:49AM -0500, Nayna wrote: > > On 11/22/22 18:21, Nayna wrote: > > > > From the perspective of our use case, we need to expose firmware > > security objects to userspace for management. Not all of the objects > > pre-exist and we would like to allow root to create them from userspace. > > > > From a unification perspective, I have considered a common location at > > /sys/firmware/security for managing any platform's security objects. And > > I've proposed a generic filesystem, which could be used by any platform > > to represent firmware security objects via /sys/firmware/security. > > > > Here are some alternatives to generic filesystem in discussion: > > > > 1. Start with a platform-specific filesystem. If more platforms would > > like to use the approach, it can be made generic. We would still have a > > common location of /sys/firmware/security and new code would live in > > arch. This is my preference and would be the best fit for our use case. > > > > 2. Use securityfs. This would mean modifying it to satisfy other use > > cases, including supporting userspace file creation. I don't know if the > > securityfs maintainer would find that acceptable. I would also still > > want some way to expose variables at /sys/firmware/security. > > > > 3. Use a sysfs-based approach. This would be a platform-specific > > implementation. However, sysfs has a similar issue to securityfs for > > file creation. When I tried it in RFC v1[1], I had to implement a > > workaround to achieve that. > > > > [1] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-nayna@linux.ibm.com/ > > > Hi Greg, > > Based on the discussions so far, is Option 1, described above, an acceptable > next step? No, as I said almost a year ago, I do not want to see platform-only filesystems going and implementing stuff that should be shared by all platforms. thanks, greg k-h
On 11/23/22 10:57, Greg Kroah-Hartman wrote: > On Wed, Nov 23, 2022 at 10:05:49AM -0500, Nayna wrote: >> On 11/22/22 18:21, Nayna wrote: >>> From the perspective of our use case, we need to expose firmware >>> security objects to userspace for management. Not all of the objects >>> pre-exist and we would like to allow root to create them from userspace. >>> >>> From a unification perspective, I have considered a common location at >>> /sys/firmware/security for managing any platform's security objects. And >>> I've proposed a generic filesystem, which could be used by any platform >>> to represent firmware security objects via /sys/firmware/security. >>> >>> Here are some alternatives to generic filesystem in discussion: >>> >>> 1. Start with a platform-specific filesystem. If more platforms would >>> like to use the approach, it can be made generic. We would still have a >>> common location of /sys/firmware/security and new code would live in >>> arch. This is my preference and would be the best fit for our use case. >>> >>> 2. Use securityfs. This would mean modifying it to satisfy other use >>> cases, including supporting userspace file creation. I don't know if the >>> securityfs maintainer would find that acceptable. I would also still >>> want some way to expose variables at /sys/firmware/security. >>> >>> 3. Use a sysfs-based approach. This would be a platform-specific >>> implementation. However, sysfs has a similar issue to securityfs for >>> file creation. When I tried it in RFC v1[1], I had to implement a >>> workaround to achieve that. >>> >>> [1] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-3-nayna@linux.ibm.com/ >>> >> Hi Greg, >> >> Based on the discussions so far, is Option 1, described above, an acceptable >> next step? > No, as I said almost a year ago, I do not want to see platform-only > filesystems going and implementing stuff that should be shared by all > platforms. Given there are no other exploiters for fwsecurityfs and there should be no platform-specific fs, would modifying sysfs now to let userspace create files cleanly be the way forward? Or, if we should strongly consider securityfs, which would result in updating securityfs to allow userspace creation of files and then expose variables via a more platform-specific directory /sys/kernel/security/pks? We want to pick the best available option and would find some hints on direction helpful before we develop the next patch. Thanks & Regards, - Nayna
On Wed, 2022-11-23 at 13:57 -0500, Nayna wrote: > > Given there are no other exploiters for fwsecurityfs and there should > be > no platform-specific fs, would modifying sysfs now to let userspace > create files cleanly be the way forward? Or, if we should strongly > consider securityfs, which would result in updating securityfs to > allow > userspace creation of files and then expose variables via a more > platform-specific directory /sys/kernel/security/pks? We want to pick > the best available option and would find some hints on direction > helpful > before we develop the next patch. Ping - it would be helpful for us to know your thoughts on this. Andrew
On Mon, Dec 12, 2022 at 11:58:56AM +1100, Andrew Donnellan wrote: > On Wed, 2022-11-23 at 13:57 -0500, Nayna wrote: > > > > Given there are no other exploiters for fwsecurityfs and there should > > be > > no platform-specific fs, would modifying sysfs now to let userspace > > create files cleanly be the way forward? Or, if we should strongly > > consider securityfs, which would result in updating securityfs to > > allow > > userspace creation of files and then expose variables via a more > > platform-specific directory /sys/kernel/security/pks? We want to pick > > the best available option and would find some hints on direction > > helpful > > before we develop the next patch. > > Ping - it would be helpful for us to know your thoughts on this. sysfs is not for userspace creation of files, you all know this :) greg k-h
diff --git a/fs/Kconfig b/fs/Kconfig index 2685a4d0d353..2a24f1c779dd 100644 --- a/fs/Kconfig +++ b/fs/Kconfig @@ -275,6 +275,7 @@ config ARCH_HAS_GIGANTIC_PAGE source "fs/configfs/Kconfig" source "fs/efivarfs/Kconfig" +source "fs/fwsecurityfs/Kconfig" endmenu diff --git a/fs/Makefile b/fs/Makefile index 4dea17840761..b945019a9bbe 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -134,6 +134,7 @@ obj-$(CONFIG_F2FS_FS) += f2fs/ obj-$(CONFIG_CEPH_FS) += ceph/ obj-$(CONFIG_PSTORE) += pstore/ obj-$(CONFIG_EFIVAR_FS) += efivarfs/ +obj-$(CONFIG_FWSECURITYFS) += fwsecurityfs/ obj-$(CONFIG_EROFS_FS) += erofs/ obj-$(CONFIG_VBOXSF_FS) += vboxsf/ obj-$(CONFIG_ZONEFS_FS) += zonefs/ diff --git a/fs/fwsecurityfs/Kconfig b/fs/fwsecurityfs/Kconfig new file mode 100644 index 000000000000..1dc2ee831eda --- /dev/null +++ b/fs/fwsecurityfs/Kconfig @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 IBM Corporation +# Author: Nayna Jain <nayna@linux.ibm.com> +# + +config FWSECURITYFS + bool "Enable the fwsecurityfs filesystem" + help + This will build fwsecurityfs filesystem which should be mounted + on /sys/firmware/security. This filesystem can be used by + platform to expose firmware-managed variables. + + If you are unsure how to answer this question, answer N. diff --git a/fs/fwsecurityfs/Makefile b/fs/fwsecurityfs/Makefile new file mode 100644 index 000000000000..5c7b76228ebb --- /dev/null +++ b/fs/fwsecurityfs/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Copyright (C) 2022 IBM Corporation +# Author: Nayna Jain <nayna@linux.ibm.com> +# +# Makefile for the firmware security filesystem + +obj-$(CONFIG_FWSECURITYFS) += fwsecurityfs.o + +fwsecurityfs-objs := super.o diff --git a/fs/fwsecurityfs/super.c b/fs/fwsecurityfs/super.c new file mode 100644 index 000000000000..99ca4da4ab63 --- /dev/null +++ b/fs/fwsecurityfs/super.c @@ -0,0 +1,263 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain <nayna@linux.ibm.com> + */ + +#include <linux/fs.h> +#include <linux/fs_context.h> +#include <linux/pagemap.h> +#include <linux/init.h> +#include <linux/namei.h> +#include <linux/magic.h> +#include <linux/fwsecurityfs.h> + +static struct super_block *fwsecsb; +static struct vfsmount *mount; +static int mount_count; +static bool fwsecurityfs_initialized; + +static void fwsecurityfs_free_inode(struct inode *inode) +{ + free_inode_nonrcu(inode); +} + +static const struct super_operations fwsecurityfs_super_operations = { + .statfs = simple_statfs, + .free_inode = fwsecurityfs_free_inode, +}; + +static int fwsecurityfs_fill_super(struct super_block *sb, + struct fs_context *fc) +{ + static const struct tree_descr files[] = {{""}}; + int rc; + + rc = simple_fill_super(sb, FWSECURITYFS_MAGIC, files); + if (rc) + return rc; + + sb->s_op = &fwsecurityfs_super_operations; + + fwsecsb = sb; + + rc = arch_fwsecurityfs_init(); + + if (!rc) + fwsecurityfs_initialized = true; + + return rc; +} + +static int fwsecurityfs_get_tree(struct fs_context *fc) +{ + return get_tree_single(fc, fwsecurityfs_fill_super); +} + +static const struct fs_context_operations fwsecurityfs_context_ops = { + .get_tree = fwsecurityfs_get_tree, +}; + +static int fwsecurityfs_init_fs_context(struct fs_context *fc) +{ + fc->ops = &fwsecurityfs_context_ops; + + return 0; +} + +static void fwsecurityfs_kill_sb(struct super_block *sb) +{ + kill_litter_super(sb); + + fwsecurityfs_initialized = false; +} + +static struct file_system_type fs_type = { + .owner = THIS_MODULE, + .name = "fwsecurityfs", + .init_fs_context = fwsecurityfs_init_fs_context, + .kill_sb = fwsecurityfs_kill_sb, +}; + +static struct dentry *fwsecurityfs_create_dentry(const char *name, umode_t mode, + u16 filesize, + struct dentry *parent, + struct dentry *dentry, void *data, + const struct file_operations *fops, + const struct inode_operations *iops) +{ + struct inode *inode; + int rc; + struct inode *dir; + struct dentry *ldentry = dentry; + + /* Calling simple_pin_fs() while initial mount in progress results in recursive + * call to mount. + */ + if (fwsecurityfs_initialized) { + rc = simple_pin_fs(&fs_type, &mount, &mount_count); + if (rc) + return ERR_PTR(rc); + } + + dir = d_inode(parent); + + /* For userspace created files, lock is already taken. */ + if (!dentry) + inode_lock(dir); + + if (!dentry) { + ldentry = lookup_one_len(name, parent, strlen(name)); + if (IS_ERR(ldentry)) + goto out; + + if (d_really_is_positive(ldentry)) { + rc = -EEXIST; + goto out1; + } + } + + inode = new_inode(dir->i_sb); + if (!inode) { + rc = -ENOMEM; + goto out1; + } + + inode->i_ino = get_next_ino(); + inode->i_mode = mode; + inode->i_atime = current_time(inode); + inode->i_mtime = current_time(inode); + inode->i_ctime = current_time(inode); + inode->i_private = data; + + if (S_ISDIR(mode)) { + inode->i_op = iops ? iops : &simple_dir_inode_operations; + inode->i_fop = &simple_dir_operations; + inc_nlink(inode); + inc_nlink(dir); + } else { + inode->i_fop = fops ? fops : &simple_dir_operations; + } + + if (S_ISREG(mode)) { + inode_lock(inode); + i_size_write(inode, filesize); + inode_unlock(inode); + } + d_instantiate(ldentry, inode); + + /* dget() here is required for userspace created files. */ + if (dentry) + dget(ldentry); + + if (!dentry) + inode_unlock(dir); + + return ldentry; + +out1: + ldentry = ERR_PTR(rc); + +out: + if (fwsecurityfs_initialized) + simple_release_fs(&mount, &mount_count); + + if (!dentry) + inode_unlock(dir); + + return ldentry; +} + +struct dentry *fwsecurityfs_create_file(const char *name, umode_t mode, + u16 filesize, struct dentry *parent, + struct dentry *dentry, void *data, + const struct file_operations *fops) +{ + if (!parent) + return ERR_PTR(-EINVAL); + + return fwsecurityfs_create_dentry(name, mode, filesize, parent, + dentry, data, fops, NULL); +} +EXPORT_SYMBOL_GPL(fwsecurityfs_create_file); + +struct dentry *fwsecurityfs_create_dir(const char *name, umode_t mode, + struct dentry *parent, + const struct inode_operations *iops) +{ + if (!parent) { + if (!fwsecsb) + return ERR_PTR(-EIO); + parent = fwsecsb->s_root; + } + + return fwsecurityfs_create_dentry(name, mode, 0, parent, NULL, NULL, + NULL, iops); +} +EXPORT_SYMBOL_GPL(fwsecurityfs_create_dir); + +static int fwsecurityfs_remove_dentry(struct dentry *dentry) +{ + struct inode *dir; + + if (!dentry || IS_ERR(dentry)) + return -EINVAL; + + dir = d_inode(dentry->d_parent); + inode_lock(dir); + if (simple_positive(dentry)) { + dget(dentry); + if (d_is_dir(dentry)) + simple_rmdir(dir, dentry); + else + simple_unlink(dir, dentry); + d_delete(dentry); + dput(dentry); + } + inode_unlock(dir); + + /* Once fwsecurityfs_initialized is set to true, calling this for + * removing files created during initial mount might result in + * imbalance of simple_pin_fs() and simple_release_fs() calls. + */ + if (fwsecurityfs_initialized) + simple_release_fs(&mount, &mount_count); + + return 0; +} + +int fwsecurityfs_remove_dir(struct dentry *dentry) +{ + if (!d_is_dir(dentry)) + return -EPERM; + + return fwsecurityfs_remove_dentry(dentry); +} +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_dir); + +int fwsecurityfs_remove_file(struct dentry *dentry) +{ + return fwsecurityfs_remove_dentry(dentry); +}; +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file); + +static int __init fwsecurityfs_init(void) +{ + int rc; + + rc = sysfs_create_mount_point(firmware_kobj, "security"); + if (rc) + return rc; + + rc = register_filesystem(&fs_type); + if (rc) { + sysfs_remove_mount_point(firmware_kobj, "security"); + return rc; + } + + return 0; +} +core_initcall(fwsecurityfs_init); +MODULE_DESCRIPTION("Firmware Security Filesystem"); +MODULE_AUTHOR("Nayna Jain"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/fwsecurityfs.h b/include/linux/fwsecurityfs.h new file mode 100644 index 000000000000..ed8f328f3133 --- /dev/null +++ b/include/linux/fwsecurityfs.h @@ -0,0 +1,29 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2022 IBM Corporation + * Author: Nayna Jain <nayna@linux.ibm.com> + */ + +#ifndef _FWSECURITYFS_H_ +#define _FWSECURITYFS_H_ + +#include <linux/ctype.h> +#include <linux/fs.h> + +struct dentry *fwsecurityfs_create_file(const char *name, umode_t mode, + u16 filesize, struct dentry *parent, + struct dentry *dentry, void *data, + const struct file_operations *fops); + +int fwsecurityfs_remove_file(struct dentry *dentry); +struct dentry *fwsecurityfs_create_dir(const char *name, umode_t mode, + struct dentry *parent, + const struct inode_operations *iops); +int fwsecurityfs_remove_dir(struct dentry *dentry); + +static int arch_fwsecurityfs_init(void) +{ + return 0; +} + +#endif /* _FWSECURITYFS_H_ */ diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h index 6325d1d0e90f..553a5fdfabce 100644 --- a/include/uapi/linux/magic.h +++ b/include/uapi/linux/magic.h @@ -53,6 +53,7 @@ #define QNX4_SUPER_MAGIC 0x002f /* qnx4 fs detection */ #define QNX6_SUPER_MAGIC 0x68191122 /* qnx6 fs detection */ #define AFS_FS_MAGIC 0x6B414653 +#define FWSECURITYFS_MAGIC 0x5345434e /* "SECM" */ #define REISERFS_SUPER_MAGIC 0x52654973 /* used by gcc */