linux-kernel - Re: [PATCH 2/2] dmi-id: add dmi/id/oem group for exporting oem strings to sysfs

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: <20160719110314.05d74b84@endymion>
Date:	Tue, 19 Jul 2016 11:03:14 +0200
From:	Jean Delvare <jdelvare@...e.de>
To:	Allen Hung <allen_hung@...l.com>
Cc:	Jean Delvare <jdelvare@...e.com>, linux-kernel@...r.kernel.org,
	Mario Limonciello <mario_limonciello@...l.com>
Subject: Re: [PATCH 2/2] dmi-id: add dmi/id/oem group for exporting oem
 strings to sysfs

Hello Allen,

On Thu, 14 Jul 2016 16:01:23 +0800, Allen Hung wrote:
> The oem strings in DMI system identification information of the BIOS have
> been parsed and stored as dmi devices in dmi_scan.c but they are not
> exported to userspace via sysfs.

They are intended for internal consumption by the kernel drivers.

> The patch intends to export oem strings to sysfs device /sys/class/dmi/id.
> As the number of oem strings are dynamic, a group "oem" is added to the
> device and the strings will be added to the group as string1, string2, ...,
> and stringN.

What is the use case? You can already get these strings easily using
dmidecode:

# dmidecode -qt 11
OEM Strings
	String 1: Dell System
	String 2: 1[05A4]
	String 3: 3[1.0]
	String 4: 12[www.dell.com]
	String 5: 14[1]
	String 6: 15[3]
	String 7:  

If needed, a dedicated option could be added to dmidecode to extract
specific OEM strings. Or existing option -s could be extended for that
purpose.

Also your code doesn't even build. I won't review this patch until I
know why it is needed, and it builds (without warning.)

One comment below though:

> 
> Signed-off-by: Allen Hung <allen_hung@...l.com>
> ---
>  drivers/firmware/dmi-id.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> index 44c0139..f284a07 100644
> --- a/drivers/firmware/dmi-id.c
> +++ b/drivers/firmware/dmi-id.c
> (...)
> +static int __init dmi_id_init_oem_attr_group(void)
> +{
> +	int i, ret;
> +	const struct dmi_device *dev;
> +	struct dmi_oem_attribute *oa, *tmp;
> +	struct device_attribute dev_attr_tmpl =
> +		__ATTR(, 0444, sys_dmi_oem_show, NULL);

I'd be very careful about permissions. OEM strings could contain pretty
much everything, including serial numbers or passwords. Making these
files world-readable doesn't strike me as the best of the ideas.

-- 
Jean Delvare
SUSE L3 Support