[PATCH] firmware/dmi: Print product serial number if any

[PATCH] firmware/dmi: Print product serial number if any

[Erwan Velu]

During the boot sequence, the kernel prints a string to identify the
running host. This is pretty handy to estimate the underlying hardware
in case of early crash.

This patch add the product serial number in this reporting if any got found,
or keep the current string. In a large fleet of servers, this
information could be useful to identify the host that generated a given trace.

Test ran with the following command : qemu-system-x86_64 -kernel arch/x86/boot/bzImage --enable-kvm -m 512  -serial stdio  -append 'console=ttyS0' -smbios type=1,serial=R90YT7WC

Prior the patch:
[    0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014

With the patch:
[    0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014, SN: R90YT7WC

Signed-off-by: Erwan Velu <e.velu@criteo.com>
---
 drivers/firmware/dmi_scan.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 0eb6b617f709..601e003c67f4 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -536,6 +536,7 @@ static void __init dmi_format_ids(char *buf, size_t len)
 {
 	int c = 0;
 	const char *board;	/* Board Name is optional */
+	const char *product_serial; /* Product serial is optional */
 
 	c += print_filtered(buf + c, len - c,
 			    dmi_get_system_info(DMI_SYS_VENDOR));
@@ -554,6 +555,14 @@ static void __init dmi_format_ids(char *buf, size_t len)
 	c += scnprintf(buf + c, len - c, " ");
 	c += print_filtered(buf + c, len - c,
 			    dmi_get_system_info(DMI_BIOS_DATE));
+
+	product_serial = dmi_get_system_info(DMI_PRODUCT_SERIAL);
+	if (product_serial) {
+		if (strlen(product_serial)) {
+			c += scnprintf(buf + c, len - c, ", SN: ");
+			c += print_filtered(buf + c, len - c, product_serial);
+		}
+	}
 }
 
 /*
-- 
2.37.3

Re: [PATCH] firmware/dmi: Print product serial number if any

[Jean Delvare]

Hi Erwan,

On Mon, 2022-10-03 at 16:24 +0200, Erwan Velu wrote:
> During the boot sequence, the kernel prints a string to identify the
> running host. This is pretty handy to estimate the underlying hardware
> in case of early crash.
> 
> This patch add the product serial number in this reporting if any got found,
> or keep the current string. In a large fleet of servers, this
> information could be useful to identify the host that generated a given trace.
> 
> Test ran with the following command : qemu-system-x86_64 -kernel arch/x86/boot/bzImage --enable-kvm -m 512  -serial stdio  -append 'console=ttyS0' -smbios type=1,serial=R90YT7WC
> 
> Prior the patch:
> [    0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014
> 
> With the patch:
> [    0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-1.fc36 04/01/2014, SN: R90YT7WC
> 
> Signed-off-by: Erwan Velu <e.velu@criteo.com>
> ---
>  drivers/firmware/dmi_scan.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>  (...)

I see the value of this change. However it raises a privacy concern. So
far, /sys/devices/virtual/dmi/id/product_serial is only readable by
root. Writing the same string to the kernel log where everybody can
read it is thus questionable.

I can think of 2 ways to solve that problem:
1* I'm not really sure why the serial numbers stored in the DMI table
were originally considered a secret. So we could change these sysfs
attributes to mode 644, and then printing them in the kernel log would
of course be OK.
2* We could make your code dependent on
CONFIG_SECURITY_DMESG_RESTRICT=y. If dmesg is restricted to root then
including the serial number there is OK, regardless of the permissions
of /sys/devices/virtual/dmi/id/product_serial .

For simplicity I'd go with option 1, but if people strongly disagree
with making serial numbers world-readable, we can go with option 2.

Now from an implementation perspective, you chose to go with
DMI_PRODUCT_SERIAL. That will work fine for vendor systems, however for
self-assembled systems, DMI_PRODUCT_SERIAL is typically unset and
DMI_BOARD_SERIAL would be more relevant. So it may make sense to
fallback to "BSN: <DMI_BOARD_SERIAL>" if DMI_PRODUCT_SERIAL is unset.
Unless you think such systems are out of scope for the intended purpose
anyway, in which case your code is OK.

Thanks,
-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH] firmware/dmi: Print product serial number if any

[Erwan Velu]

[...]
> I see the value of this change. However it raises a privacy concern. So
> far, /sys/devices/virtual/dmi/id/product_serial is only readable by
> root. Writing the same string to the kernel log where everybody can
> read it is thus questionable.
>
> I can think of 2 ways to solve that problem:
> 1* I'm not really sure why the serial numbers stored in the DMI table
> were originally considered a secret. So we could change these sysfs
> attributes to mode 644, and then printing them in the kernel log would
> of course be OK.
> 2* We could make your code dependent on
> CONFIG_SECURITY_DMESG_RESTRICT=y. If dmesg is restricted to root then
> including the serial number there is OK, regardless of the permissions
> of /sys/devices/virtual/dmi/id/product_serial .
>
> For simplicity I'd go with option 1, but if people strongly disagree
> with making serial numbers world-readable, we can go with option 2.


Yeah I don't really see why showing a serial host in a dmesg is a
security problem.
We already expose other serials like disk or mac addresses which could
be considered private in the same way.


> Now from an implementation perspective, you chose to go with
> DMI_PRODUCT_SERIAL. That will work fine for vendor systems, however for
> self-assembled systems, DMI_PRODUCT_SERIAL is typically unset and
> DMI_BOARD_SERIAL would be more relevant. So it may make sense to
> fallback to "BSN: <DMI_BOARD_SERIAL>" if DMI_PRODUCT_SERIAL is unset.
> Unless you think such systems are out of scope for the intended purpose
> anyway, in which case your code is OK.

Note we have already the same issue with DMI_BIOS_VERSION vs DMI_BIOS_RELEASE.
On HPE system, reported the VERSION is useless as it points to the
motherboard type (U30/U32..) while the release field is the 'version'
like 2.56. On ODM systems, that is the opposite. \o/