linux-kernel - [RFC PATCH] firmware: qemu_fw_cfg.c: default to listing user-facing items only

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>] [day] [month] [year] [list]

Message-ID: <20160503183340.GD18050@HEDWIG.INI.CMU.EDU>
Date:	Tue, 3 May 2016 14:33:48 -0400
From:	Gabriel Somlo <somlo@....edu>
To:	linux-kernel@...r.kernel.org
Cc:	gregkh@...uxfoundation.org, mst@...hat.com, lersek@...hat.com,
	kraxel@...hat.com, matt@...eblueprint.co.uk, luto@...capital.net,
	pbonzini@...hat.com, armbru@...hat.com, qemu-devel@...gnu.org
Subject: [RFC PATCH] firmware: qemu_fw_cfg.c: default to listing user-facing
 items only

Per the QEMU fw_cfg documentation, items targeted at guest-side
userspace should have names beginning with the string "opt/".
This patch limits the default sysfs fw_cfg listing to items
named "opt/*".

The "opt/" prefix on a fw_cfg item name may be interpreted as
being analogous to bit 2 in ACPI's _STA object return value
(i.e., "show in UI").

A full listing of all fw_cfg items can still be made available
by enabling the 'list_all' module parameter, also introduced by
this patch.

Suggested-by: Michael S. Tsirkin <mst@...hat.com>
Signed-off-by: Gabriel Somlo <somlo@....edu>
---

Sorry it took me this long to whip together the few lines of code
below -- end-of-semester craziness happened, and I'm just beginning
to recover... :)

I've (somewhat reluctantly) modified the fw-cfg sysfs driver to
only show blobs named "opt/*" by default, at Michael's request.

Even though my instinct tells me hardcoding "opt/" in the source
feels a bit like mixing in policy, the argument that "opt/" is
analogous to the ACPI "hide from u/i" bit does make a lot of sense.

Personally, I can live with non-"opt/" blobs being hidden by default,
particularly since I'm still allowing them to be listed via the
"list-all" module parameter.

I'm cc-ing everyone I can remember showing any interest in this
driver and/or offering help and feedback, for a chance to comment
(with apologies if I left out anyone).

Absent any further objections, I'm personally OK with defaulting
to "opt/"-only blobs being shown to userspace.

Thanks again for all the help and feedback,
  --Gabriel

 drivers/firmware/qemu_fw_cfg.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index d999fe3..c580b73 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -37,6 +37,10 @@ MODULE_AUTHOR("Gabriel L. Somlo <somlo@....edu>");
 MODULE_DESCRIPTION("QEMU fw_cfg sysfs support");
 MODULE_LICENSE("GPL");

+static int modparam_list_all;
+module_param_named(list_all, modparam_list_all, int, 0444);
+MODULE_PARM_DESC(list_all, "List all fw-cfg items under by_name sysfs folder.");
+
 /* selector key values for "well-known" fw_cfg entries */
 #define FW_CFG_SIGNATURE  0x00
 #define FW_CFG_ID         0x01
@@ -452,6 +456,10 @@ static int fw_cfg_register_file(const struct fw_cfg_file *f)
 	int err;
 	struct fw_cfg_sysfs_entry *entry;

+	/* skip listing item if name does not begin with "opt/" */
+	if (!modparam_list_all && strncmp(f->name, "opt/", 4))
+		return 0;
+
 	/* allocate new entry */
 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
-- 
2.4.11