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@xxxxxxxxxx>
> ---
>  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