linux-kernel - [PATCH v6 2/3] sysfs: Add a attr_is_visible function to attribute

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230817235810.596458-2-alistair.francis@wdc.com>
Date:   Thu, 17 Aug 2023 19:58:09 -0400
From:   Alistair Francis <alistair23@...il.com>
To:     bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        Jonathan.Cameron@...wei.com, lukas@...ner.de
Cc:     alex.williamson@...hat.com, christian.koenig@....com,
        kch@...dia.com, gregkh@...uxfoundation.org, logang@...tatee.com,
        linux-kernel@...r.kernel.org, alistair23@...il.com,
        chaitanyak@...dia.com, rdunlap@...radead.org,
        Alistair Francis <alistair.francis@....com>
Subject: [PATCH v6 2/3] sysfs: Add a attr_is_visible function to attribute_group

When creating an attribute group, if it is named a subdirectory it is
created and the sysfs files are placed into that subdirectory.  If no
files are created, normally the directory would still be present, but it
would be empty.

This can be confusing for users, as it appears the feature is avaliable
as there is a directory, but it isn't supported by the hardware or the
kernel.

One way to fix this is to remove directories that don't contain any
files, such as [1]. The problem with this is that there are currently
lots of users in the kernel who expect the group to remain even if
empty, as they dynamically add/merge properties later.

The documentation for sysfs_merge_group() specifically says

    This function returns an error if the group doesn't exist or any of the
    files already exist in that group, in which case none of the new files
    are created.

So just not adding the group if it's empty doesn't work, at least not
with the code we currently have. The code can be changed to support
this, but it is difficult to determine how this will affect existing use
cases.

This approach instead adds a new function pointer, attr_is_visible(), to
`struct attribute_group` which can be set if the user wants to filter
the avaliablility of the function.

This matches the .is_visible() function pointer that already exists and
is commonly used. This approach provides greater control over if the
directory should be visible or not.

This will be used by the PCIe DOE sysfs attributes to kind the directory
on devices that don't support DOE.

1: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=debugfs_cleanup&id=f670945dfbaf353fe068544c31e3fa45575da5b5

Signed-off-by: Alistair Francis <alistair.francis@....com>
---
v6:
 - Add patch

 fs/sysfs/group.c      | 12 +++++++++++-
 include/linux/sysfs.h |  6 ++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 138676463336..34afd5becdbe 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -111,6 +111,7 @@ static int internal_create_group(struct kobject *kobj, int update,
 	kuid_t uid;
 	kgid_t gid;
 	int error;
+	umode_t mode;
 
 	if (WARN_ON(!kobj || (!update && !kobj->sd)))
 		return -EINVAL;
@@ -125,6 +126,15 @@ static int internal_create_group(struct kobject *kobj, int update,
 		return 0;
 	}
 
+	if (grp->attr_is_visible) {
+		mode = grp->attr_is_visible(kobj);
+
+		if (mode == 0)
+			return 0;
+	} else {
+		mode = S_IRWXU | S_IRUGO | S_IXUGO;
+	}
+
 	kobject_get_ownership(kobj, &uid, &gid);
 	if (grp->name) {
 		if (update) {
@@ -136,7 +146,7 @@ static int internal_create_group(struct kobject *kobj, int update,
 			}
 		} else {
 			kn = kernfs_create_dir_ns(kobj->sd, grp->name,
-						  S_IRWXU | S_IRUGO | S_IXUGO,
+						  mode,
 						  uid, gid, kobj, NULL);
 			if (IS_ERR(kn)) {
 				if (PTR_ERR(kn) == -EEXIST)
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..808e7fc0ca57 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -63,6 +63,11 @@ do {							\
  * @name:	Optional: Attribute group name
  *		If specified, the attribute group will be created in
  *		a new subdirectory with this name.
+ * @attr_is_visible:	Optional: Function to return permissions
+ *		associated with the attribute group. Only read/write
+ *		permissions as well as SYSFS_PREALLOC are accepted. Must
+ *		return 0 if an attribute is not visible. The returned value
+ *		will replace static permissions defined in struct attribute.
  * @is_visible:	Optional: Function to return permissions associated with an
  *		attribute of the group. Will be called repeatedly for each
  *		non-binary attribute in the group. Only read/write
@@ -83,6 +88,7 @@ do {							\
  */
 struct attribute_group {
 	const char		*name;
+	umode_t			(*attr_is_visible)(struct kobject *);
 	umode_t			(*is_visible)(struct kobject *,
 					      struct attribute *, int);
 	umode_t			(*is_bin_visible)(struct kobject *,
-- 
2.41.0