[PATCH] dmi: export dmi data through debugfs

[PATCH] dmi: export dmi data through debugfs

[Olof Johansson]

I've found this quite useful since it allows dmidecode to run without
root privileges using --from-dump to read this file instead (note:
dmidecode needs a change to fall back from mmap to regular reads, since
debugfs exports can't be mmapped()).

It also changes the calling convention of dmi_present somewhat, since the
16-byte EPS header is needed in the exported file.

Signed-off-by: Olof Johansson <olof@lixom.net>

---
 drivers/firmware/dmi_scan.c |   94 ++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 83 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index d464672..26141e1 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -5,6 +5,7 @@
 #include <linux/dmi.h>
 #include <linux/efi.h>
 #include <linux/bootmem.h>
+#include <linux/debugfs.h>
 #include <asm/dmi.h>
 
 /*
@@ -99,6 +100,73 @@ static u32 dmi_base;
 static u16 dmi_len;
 static u16 dmi_num;
 
+#ifdef CONFIG_DEBUG_FS
+static struct debugfs_blob_wrapper dmi_blob;
+
+/* Export a copy of the DMI blob through debugfs. Just concatenate the
+ * header with the main blob, and make the pointer the file offset of the
+ * main blob (i.e. 32 bytes).
+ */
+static int __init dmi_debug_setup(const char __iomem *p)
+{
+	u8 *blob, *buf, tmp;
+	int i;
+
+	blob = dmi_alloc(dmi_len + 32);
+	if (!blob)
+		return -ENOMEM;
+
+	memcpy_fromio(blob, p, 32);
+
+	buf = dmi_ioremap(dmi_base, dmi_len);
+	memcpy_fromio(blob + 32, buf, dmi_len);
+	dmi_iounmap(buf, dmi_len);
+
+	/* 32 is the offset into the file  */
+	blob[24] = 32;
+	blob[25] = 0;
+	blob[26] = 0;
+	blob[27] = 0;
+
+	/* First checksum the IEPS */
+	blob[21] = 0;
+	tmp = 0;
+	for (i = 0x10; i < 0x1f; i++)
+		tmp += blob[i];
+	blob[21] = -tmp;
+
+	/* Then the EPS */
+	blob[4] = 0;
+	tmp = 0;
+	for (i = 0; i < blob[5]; i++)
+		tmp += blob[i];
+	blob[4] = -tmp;
+
+	dmi_blob.data = blob;
+	dmi_blob.size = dmi_len + 32;
+
+	return 0;
+}
+
+static int __init dmi_debugfs_create(void)
+{
+	void *ret = ERR_PTR(-ENODEV);
+
+	if (dmi_blob.data)
+		ret = debugfs_create_blob("dmidump", 0444, NULL, &dmi_blob);
+
+	return PTR_ERR(ret);
+}
+late_initcall(dmi_debugfs_create);
+
+#else
+
+static int __init dmi_debug_setup(const char __iomem *p)
+{
+	return 0;
+}
+#endif /* CONFIG_DEBUGFS */
+
 static int __init dmi_walk_early(void (*decode)(const struct dmi_header *,
 		void *))
 {
@@ -338,26 +406,30 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
 
 static int __init dmi_present(const char __iomem *p)
 {
-	u8 buf[15];
+	u8 buf[31];
 
-	memcpy_fromio(buf, p, 15);
-	if ((memcmp(buf, "_DMI_", 5) == 0) && dmi_checksum(buf)) {
-		dmi_num = (buf[13] << 8) | buf[12];
-		dmi_len = (buf[7] << 8) | buf[6];
-		dmi_base = (buf[11] << 24) | (buf[10] << 16) |
-			(buf[9] << 8) | buf[8];
+	memcpy_fromio(buf, p, 31);
+	/* _DMI_ string starts 16 bytes in. */
+	if ((memcmp(buf+16, "_DMI_", 5) == 0) && dmi_checksum(buf+16)) {
+		dmi_num = (buf[29] << 8) | buf[28];
+		dmi_len = (buf[23] << 8) | buf[22];
+		dmi_base = (buf[27] << 24) | (buf[26] << 16) |
+			(buf[25] << 8) | buf[24];
 
 		/*
 		 * DMI version 0.0 means that the real version is taken from
 		 * the SMBIOS version, which we don't know at this point.
 		 */
-		if (buf[14] != 0)
+		if (buf[30] != 0)
 			printk(KERN_INFO "DMI %d.%d present.\n",
-			       buf[14] >> 4, buf[14] & 0xF);
+			       buf[30] >> 4, buf[30] & 0xf);
 		else
 			printk(KERN_INFO "DMI present.\n");
-		if (dmi_walk_early(dmi_decode) == 0)
+		if (dmi_walk_early(dmi_decode) == 0) {
+			dmi_debug_setup(p);
+
 			return 0;
+		}
 	}
 	return 1;
 }
@@ -379,7 +451,7 @@ void __init dmi_scan_machine(void)
 		if (p == NULL)
 			goto error;
 
-		rc = dmi_present(p + 0x10); /* offset of _DMI_ string */
+		rc = dmi_present(p);
 		dmi_iounmap(p, 32);
 		if (!rc) {
 			dmi_available = 1;
-- 
1.7.0.4

Re: [PATCH] dmi: export dmi data through debugfs

[Jean Delvare]

Hi Olaf,

On Tue, 28 Sep 2010 16:12:46 -0500, Olof Johansson wrote:
> I've found this quite useful since it allows dmidecode to run without
> root privileges using --from-dump to read this file instead

This is a bad idea. We do NOT want every user to have access to all the
DMI information. There is sensitive information in there (serial
numbers and UUIDs, and possibly even more sensitive data in
OEM-specific records.) If you look in /sys/class/dmi/id/, you'll see
that files board_serial, chassis_serial, product_serial and
product_uuid are only readable by root exactly for this reason.

So this is a NACK from me, sorry.

-- 
Jean Delvare

Re: [PATCH] dmi: export dmi data through debugfs

[Olof Johansson]

On Wed, Sep 29, 2010 at 09:34:03AM +0200, Jean Delvare wrote:
> Hi Olaf,
> 
> On Tue, 28 Sep 2010 16:12:46 -0500, Olof Johansson wrote:
> > I've found this quite useful since it allows dmidecode to run without
> > root privileges using --from-dump to read this file instead
> 
> This is a bad idea. We do NOT want every user to have access to all the
> DMI information. There is sensitive information in there (serial
> numbers and UUIDs, and possibly even more sensitive data in
> OEM-specific records.) If you look in /sys/class/dmi/id/, you'll see
> that files board_serial, chassis_serial, product_serial and
> product_uuid are only readable by root exactly for this reason.

> So this is a NACK from me, sorry.

So how about a change to mode 0400 on the debugfs file then? It's
still better than having a userspace tool dig around /dev/mem for the
information.


-Olof

Re: [PATCH] dmi: export dmi data through debugfs

[Jean Delvare]

On Wed, 29 Sep 2010 09:53:30 -0500, Olof Johansson wrote:
> On Wed, Sep 29, 2010 at 09:34:03AM +0200, Jean Delvare wrote:
> > Hi Olaf,
> > 
> > On Tue, 28 Sep 2010 16:12:46 -0500, Olof Johansson wrote:
> > > I've found this quite useful since it allows dmidecode to run without
> > > root privileges using --from-dump to read this file instead
> > 
> > This is a bad idea. We do NOT want every user to have access to all the
> > DMI information. There is sensitive information in there (serial
> > numbers and UUIDs, and possibly even more sensitive data in
> > OEM-specific records.) If you look in /sys/class/dmi/id/, you'll see
> > that files board_serial, chassis_serial, product_serial and
> > product_uuid are only readable by root exactly for this reason.
> 
> > So this is a NACK from me, sorry.
> 
> So how about a change to mode 0400 on the debugfs file then?

This is a requirement, yes.

> It's
> still better than having a userspace tool dig around /dev/mem for the
> information.

How is that better, please? Unless /dev/mem is going away, I don't see
any benefit. "Digging around" is exactly what the kernel is doing to
get the information. dmidecode has been around for over 7 years now,
it's always been reading its information from /dev/mem, and while there
has been some technical challenge with this (mmap vs. read) it has been
solved long ago.

And we now have an option limiting what can be accessed
through /dev/mem (CONFIG_STRICT_DEVMEM), so we should be safe.

The only objection I ever heard is that one had to be rootto run
dmidecode, but there's no way to address this, which is why the
relevant DMI strings are now exposed through sysfs. If there's a string
you consider useful which is missing there, you could just add it.

Now if you really insist on exposing the whole DMI table through sysfs,
I can't prevent you from doing that. After all, ACPI already exposes
its tables under /sys/firmware/acpi/tables (mode 0400). But then you'd
rather expose the DMI entry point and tables
under /sys/firmware/dmi/tables for consistency, rather than using
debugfs. But again, I don't think it is adding any value over what we
already have.

Please note that anyway, dmidecode is not going to stop getting the
table from /dev/mem, because it is used on several non-Linux operating
systems (all the BSDs in particular) which do not have sysfs.

-- 
Jean Delvare

Re: [PATCH] dmi: export dmi data through debugfs

[Alan Cox]

> Now if you really insist on exposing the whole DMI table through sysfs,
> I can't prevent you from doing that. After all, ACPI already exposes
> its tables under /sys/firmware/acpi/tables (mode 0400). But then you'd
> rather expose the DMI entry point and tables
> under /sys/firmware/dmi/tables for consistency, rather than using
> debugfs. But again, I don't think it is adding any value over what we
> already have.

If you really the DMI data generally available run dmidecode in the boot
scripts directed to a file. It even has a dump mode for this.

Alan

Re: [PATCH] dmi: export dmi data through debugfs

[Jean Delvare]

Hi Alan,

On Wed, 29 Sep 2010 22:28:04 +0100, Alan Cox wrote:
> > Now if you really insist on exposing the whole DMI table through sysfs,
> > I can't prevent you from doing that. After all, ACPI already exposes
> > its tables under /sys/firmware/acpi/tables (mode 0400). But then you'd
> > rather expose the DMI entry point and tables
> > under /sys/firmware/dmi/tables for consistency, rather than using
> > debugfs. But again, I don't think it is adding any value over what we
> > already have.
> 
> If you really the DMI data generally available run dmidecode in the boot
> scripts directed to a file. It even has a dump mode for this.

Dump mode? Do you mean dmidecode --dump, dmidecode --dump-bin, or
something of your own?

-- 
Jean Delvare

Re: [PATCH] dmi: export dmi data through debugfs

[Olof Johansson]

On Wed, Sep 29, 2010 at 10:28:04PM +0100, Alan Cox wrote:
> > Now if you really insist on exposing the whole DMI table through sysfs,
> > I can't prevent you from doing that. After all, ACPI already exposes
> > its tables under /sys/firmware/acpi/tables (mode 0400). But then you'd
> > rather expose the DMI entry point and tables
> > under /sys/firmware/dmi/tables for consistency, rather than using
> > debugfs. But again, I don't think it is adding any value over what we
> > already have.
> 
> If you really the DMI data generally available run dmidecode in the boot
> scripts directed to a file. It even has a dump mode for this.

Sure. I didn't think a trivial patch like this would get pushback,
but that's a workable fallback for my use case.


Thanks,

-Olof

Re: [PATCH] dmi: export dmi data through debugfs

[Bjorn Helgaas]

On Wednesday, September 29, 2010 09:11:18 am Jean Delvare wrote:
> On Wed, 29 Sep 2010 09:53:30 -0500, Olof Johansson wrote:
> > On Wed, Sep 29, 2010 at 09:34:03AM +0200, Jean Delvare wrote:
> > > Hi Olaf,
> > > 
> > > On Tue, 28 Sep 2010 16:12:46 -0500, Olof Johansson wrote:
> > > > I've found this quite useful since it allows dmidecode to run without
> > > > root privileges using --from-dump to read this file instead
> > > 
> > > This is a bad idea. We do NOT want every user to have access to all the
> > > DMI information. There is sensitive information in there (serial
> > > numbers and UUIDs, and possibly even more sensitive data in
> > > OEM-specific records.)

If DMI has sensitive information, then I agree, we shouldn't expose
the whole table to non-root users.  I don't know the original motivation
for allowing non-root access for dmidecode -- maybe it could be addressed
by making dmidecode smart enough to look at /sys/class/dmi/id/* when
running as a non-root user?

> > It's
> > still better than having a userspace tool dig around /dev/mem for the
> > information.

I don't like /dev/mem either.  If someone's making an effort to remove
/dev/mem, this might be one piece.  But I don't know whether that's
feasible.

> Now if you really insist on exposing the whole DMI table through sysfs,
> I can't prevent you from doing that. After all, ACPI already exposes
> its tables under /sys/firmware/acpi/tables (mode 0400). But then you'd
> rather expose the DMI entry point and tables
> under /sys/firmware/dmi/tables for consistency, rather than using
> debugfs.

If you do pursue this on the grounds of migrating away from /dev/mem,
I like the idea of making it similar to what ACPI does.

Bjorn

Re: [PATCH] dmi: export dmi data through debugfs

[Andi Kleen]

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

>> Now if you really insist on exposing the whole DMI table through sysfs,
>> I can't prevent you from doing that. After all, ACPI already exposes
>> its tables under /sys/firmware/acpi/tables (mode 0400). But then you'd
>> rather expose the DMI entry point and tables
>> under /sys/firmware/dmi/tables for consistency, rather than using
>> debugfs. But again, I don't think it is adding any value over what we
>> already have.
>
> If you really the DMI data generally available run dmidecode in the boot
> scripts directed to a file. It even has a dump mode for this.

FWIW i have been pondering to put DMI into sysfs too. One of the reasons
mcelog has to start up as root is that it needs to get these tables
out of /dev/mem.

Just having a binary file to read would be fine though, no need to 
do decoding.

-andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH] dmi: export dmi data through debugfs

[Jean Delvare]

Hi Andi,

On Thu, 30 Sep 2010 19:34:57 +0200, Andi Kleen wrote:
> Alan Cox <alan@lxorguk.ukuu.org.uk> writes:
> 
> >> Now if you really insist on exposing the whole DMI table through sysfs,
> >> I can't prevent you from doing that. After all, ACPI already exposes
> >> its tables under /sys/firmware/acpi/tables (mode 0400). But then you'd
> >> rather expose the DMI entry point and tables
> >> under /sys/firmware/dmi/tables for consistency, rather than using
> >> debugfs. But again, I don't think it is adding any value over what we
> >> already have.
> >
> > If you really the DMI data generally available run dmidecode in the boot
> > scripts directed to a file. It even has a dump mode for this.
> 
> FWIW i have been pondering to put DMI into sysfs too. One of the reasons
> mcelog has to start up as root is that it needs to get these tables
> out of /dev/mem.
> 
> Just having a binary file to read would be fine though, no need to 
> do decoding.

Out of curiosity, what exactly does mcelog need out of the DMI data?

-- 
Jean Delvare