[PATCH] sysfs: Introduce is_group_visible() for attribute_groups

[Dan Williams]

Add a method to 'struct attribute_group' to determine the visibility of
an entire named sysfs group relative to the state of its parent kobject.

The motivation for this capability is to centralize PCI device
authentication in the PCI core with a named sysfs group while keeping
that group hidden for devices and platforms that do not meet the
requirements. In a PCI topology, most devices will not support
authentication, a small subset will support just PCI CMA (Component
Measurement and Authentication), a smaller subset will support PCI CMA +
PCIe IDE (Link Integrity and Encryption), and only next generation
server hosts will start to include a platform TSM (TEE Security
Manager).

Without this capability the alternatives are:

- Check if all attributes are invisible and if so, hide the directory.
  Beyond trouble getting this to work [1], this is an ABI change for
  scenarios if userspace happens to depend on group visibility absent any
  attributes. I.e. this new capability avoids regression since it does
  not retroactively apply to existing cases.

- Publish an empty /sys/bus/pci/devices/$pdev/tsm/ directory for all PCI
  devices (i.e. for the case when TSM platform support is present, but
  device support is absent). Unfortunate that this will be a vestigial
  empty directory in the vast majority of cases.

- Reintroduce usage of runtime calls to sysfs_{create,remove}_group()
  in the PCI core. Bjorn has already indicated that he does not want to
  see any growth of pci_sysfs_init() [2].

- Drop the named group and simulate a directory by prefixing all
  TSM-related attributes with "tsm_". Unfortunate to not use the naming
  capability of a sysfs group as intended.

The downside of the alternatives seem worse than the maintenance burden
of this new capability. Otherwise, this facility also brings support for
changing permissions on sysfs directories from the 0755 default for
potential cases where only root is expected to be able to enumerate.
That may prove useful as PCI sysfs picks up more security sensitive
enumeration.

The size increase of 'struct attribute_group' is a concern. Longer term
this could be reduced by consolidating the 3 is_visible() callbacks into
one that takes a parameter for "attr", "bin_attr", or "group". For now
the support is gated behind CONFIG_SYSFS_GROUP_VISIBLE so it can be
compiled out when not needed.

Link: https://lore.kernel.org/all/2024012321-envious-procedure-4a58@gregkh/ [1]
Link: https://lore.kernel.org/linux-pci/20231019200110.GA1410324@bhelgaas/ [2]
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
 fs/sysfs/Kconfig      |  3 +++
 fs/sysfs/group.c      | 50 +++++++++++++++++++++++++++++++++++--------
 include/linux/sysfs.h | 34 +++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/fs/sysfs/Kconfig b/fs/sysfs/Kconfig
index b0fe1cce33a0..d5de8e54a06f 100644
--- a/fs/sysfs/Kconfig
+++ b/fs/sysfs/Kconfig
@@ -23,3 +23,6 @@ config SYSFS
 	example, "root=03:01" for /dev/hda1.
 
 	Designers of embedded systems may wish to say N here to conserve space.
+
+config SYSFS_GROUP_VISIBLE
+	bool
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 138676463336..0a977064e118 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -127,16 +127,36 @@ static int internal_create_group(struct kobject *kobj, int update,
 
 	kobject_get_ownership(kobj, &uid, &gid);
 	if (grp->name) {
+		umode_t mode = S_IRWXU | S_IRUGO | S_IXUGO;
+
+		if (has_group_visible(grp))
+			mode = is_group_visible(grp, kobj);
+
 		if (update) {
 			kn = kernfs_find_and_get(kobj->sd, grp->name);
 			if (!kn) {
-				pr_warn("Can't update unknown attr grp name: %s/%s\n",
-					kobj->name, grp->name);
-				return -EINVAL;
+				if (!has_group_visible(grp)) {
+					pr_warn("Can't update unknown attr grp name: %s/%s\n",
+						kobj->name, grp->name);
+					return -EINVAL;
+				}
+				/* may have been invisible prior to this update */
+				update = 0;
+			} else {
+				/* need to change the visibility of the entire group */
+				sysfs_remove_group(kobj, grp);
+				if (mode == 0) {
+					kernfs_put(kn);
+					return 0;
+				}
+				update = 0;
 			}
-		} else {
-			kn = kernfs_create_dir_ns(kobj->sd, grp->name,
-						  S_IRWXU | S_IRUGO | S_IXUGO,
+		}
+
+		if (!update) {
+			if (mode == 0)
+				return 0;
+			kn = kernfs_create_dir_ns(kobj->sd, grp->name, mode,
 						  uid, gid, kobj, NULL);
 			if (IS_ERR(kn)) {
 				if (PTR_ERR(kn) == -EEXIST)
@@ -262,6 +282,20 @@ int sysfs_update_group(struct kobject *kobj,
 }
 EXPORT_SYMBOL_GPL(sysfs_update_group);
 
+static void warn_group_not_found(const struct attribute_group *grp,
+				 struct kobject *kobj)
+{
+	if (has_group_visible(grp)) {
+		/* may have never been created */
+		pr_debug("sysfs group '%s' not found for kobject '%s'\n",
+			 grp->name, kobject_name(kobj));
+		return;
+	}
+
+	WARN(1, KERN_WARNING "sysfs group '%s' not found for kobject '%s'\n",
+	     grp->name, kobject_name(kobj));
+}
+
 /**
  * sysfs_remove_group: remove a group from a kobject
  * @kobj:	kobject to remove the group from
@@ -279,9 +313,7 @@ void sysfs_remove_group(struct kobject *kobj,
 	if (grp->name) {
 		kn = kernfs_find_and_get(parent, grp->name);
 		if (!kn) {
-			WARN(!kn, KERN_WARNING
-			     "sysfs group '%s' not found for kobject '%s'\n",
-			     grp->name, kobject_name(kobj));
+			warn_group_not_found(grp, kobj);
 			return;
 		}
 	} else {
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index b717a70219f6..31f3dd6fc946 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -77,6 +77,12 @@ do {							\
  *		return 0 if a binary attribute is not visible. The returned
  *		value will replace static permissions defined in
  *		struct bin_attribute.
+ * @is_group_visible:
+ * 		Optional: Function to return permissions associated with
+ * 		directory created for named groups. When this returns 0
+ * 		all attributes are removed on update, or otherwise the
+ * 		directory is not created in the first instance. When not
+ * 		specified the default permissions of 0755 are applied.
  * @attrs:	Pointer to NULL terminated list of attributes.
  * @bin_attrs:	Pointer to NULL terminated list of binary attributes.
  *		Either attrs or bin_attrs or both must be provided.
@@ -87,10 +93,38 @@ struct attribute_group {
 					      struct attribute *, int);
 	umode_t			(*is_bin_visible)(struct kobject *,
 						  struct bin_attribute *, int);
+#ifdef CONFIG_SYSFS_GROUP_VISIBLE
+	umode_t			(*is_group_visible)(struct kobject *);
+#endif
+
 	struct attribute	**attrs;
 	struct bin_attribute	**bin_attrs;
 };
 
+#ifdef CONFIG_SYSFS_GROUP_VISIBLE
+static inline bool has_group_visible(const struct attribute_group *grp)
+{
+	return grp->is_group_visible != NULL;
+}
+
+static inline umode_t is_group_visible(const struct attribute_group *grp,
+				       struct kobject *kobj)
+{
+	return grp->is_group_visible(kobj);
+}
+#else
+static inline bool has_group_visible(const struct attribute_group *grp)
+{
+	return false;
+}
+
+static inline umode_t is_group_visible(const struct attribute_group *grp,
+				       struct kobject *kobj)
+{
+	return 0755;
+}
+#endif
+
 /*
  * Use these macros to make defining attributes easier.
  * See include/linux/device.h for examples..
-- 
2.43.0