From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752598AbcG2J7x (ORCPT ); Fri, 29 Jul 2016 05:59:53 -0400 Received: from esa8.dell-outbound.iphmx.com ([68.232.149.218]:33240 "EHLO esa8.dell-outbound.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751095AbcG2J7u (ORCPT ); Fri, 29 Jul 2016 05:59:50 -0400 DomainKey-Signature: s=smtpout; d=dell.com; c=simple; q=dns; h=Received:X-LoopCount0:X-IronPort-AV:Subject:To: References:Cc:From:Message-ID:Date:User-Agent: MIME-Version:In-Reply-To:Content-Type: Content-Transfer-Encoding; b=t6qgyIzed2SPOMJGE+htrgQ5lx1sUghI/jkSTui06efmy52aReLvBTSV lkfQ7qffqiRzCcFRdC1DSBlRUgR2fyI0GSm7IDU+2WC2sabEjf+4oA4bq roSvvKNVw9XKa2daCb1X2yOuIWmglybfXu6ciMMP6qla12yc+4PPOXf2r E=; X-LoopCount0: from 10.106.62.76 X-IronPort-AV: E=Sophos;i="5.28,438,1464670800"; d="scan'208";a="1003062538" Subject: Re: [PATCH 2/2] dmi-id: add dmi/id/oem group for exporting oem strings to sysfs To: "Limonciello, Mario" , Jean Delvare References: <1468483283-84766-1-git-send-email-allen_hung@dell.com> <1468483283-84766-3-git-send-email-allen_hung@dell.com> <20160719110314.05d74b84@endymion> Cc: Jean Delvare , "linux-kernel@vger.kernel.org" From: Allen Hung Message-ID: <579B2912.8070201@dell.com> Date: Fri, 29 Jul 2016 17:59:46 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/27/2016 05:03 AM, Limonciello, Mario wrote: >> -----Original Message----- >> From: Limonciello, Mario >> Sent: Tuesday, July 19, 2016 9:48 AM >> To: 'Jean Delvare' ; Hung, Allen >> Cc: Jean Delvare ; linux-kernel@vger.kernel.org >> Subject: RE: [PATCH 2/2] dmi-id: add dmi/id/oem group for exporting oem >> strings to sysfs >> >> Hi Jean, >> >> I worked with Allen on this concept, so I've got some comments below. >> >>> -----Original Message----- >>> From: Jean Delvare [mailto:jdelvare@suse.de] >>> Sent: Tuesday, July 19, 2016 4:03 AM >>> To: Hung, Allen >>> Cc: Jean Delvare ; linux-kernel@vger.kernel.org; >>> Limonciello, Mario >>> 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. >> >> The main purpose was to be able to parse these easily from userspace >> without needing dmidecode installed and handling its output >> (with tools such as grep, sed, and awk). >> >> For example in an initramfs, typically dmidecode isn't included, but there >> is value to being able to make decisions on things related to the values of >> those OEM strings. >> >> Instead this allows userspace to iterate the oem/ directory and directly >> look at the values of these strings. >> >>> >>> 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.) >>> >> >> Allen had a mistake in that submission when he was refactoring it prior to >> LKML submission. >> He resubmitted it the next day fixing that mistake: >> https://patchwork.kernel.org/patch/9231473/ >> >>> One comment below though: >>> >>>> >>>> Signed-off-by: Allen Hung >>>> --- >>>> 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. >>> >> >> At least on Dell systems, the values in these strings are OK to be world >> readable, but I understand this concern and agree that Allen should adjust >> these permissions in the next version if you agree with the concept of this >> patch. >> >> Thanks, > > Hi jean, > > Did you have any comments about Allen's updated patch or my above > comments? > > If necessary, Allen can resend with the fix to OEM strings permissions > and we can discuss further then. > > Thanks, > Hi Jean, I didn't send my earlier fix beginning with "PATCH v2" so it may bring up some confusing. I am sorry about it. I have just resent it with prefix "PATCH v2" and the OEM string permissions still remain what they were in my first submission. Hope we can get the feedbacks from you. Regards, Allen