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
| ||
|
Date: Wed, 23 Feb 2022 11:26:40 +0200 From: Nandor Han <nandor.han@...sala.com> To: Jonathan Cameron <Jonathan.Cameron@...wei.com> Cc: Jonathan Cameron <jic23@...nel.org>, lars@...afoo.de, linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [RFC PATCH] iio: core: provide a default value `label` property On 2/22/22 18:36, Jonathan Cameron wrote: > On Tue, 22 Feb 2022 09:42:12 +0200 > Nandor Han <nandor.han@...sala.com> wrote: > >> On 2/20/22 15:18, Jonathan Cameron wrote: >>> On Wed, 16 Feb 2022 15:56:04 +0200 >>> Nandor Han <nandor.han@...sala.com> wrote: >>> >> >> Thanks for reviewing the patch and provide feed back. >> >>>> The label property is used to correctly identify the same IIO device >>>> over reboots. The implementation requires that a value will be provided >>>> through device-tree. This sometime could requires many changes to >>>> device-trees when multiple devices want to use the label property. >>>> In order to prevent this, we could use the device-tree node >>>> name as default value. The device-tree node name is unique and >>>> also reflects the device which makes it a good choice as default value. >>>> This change is backward compatible since doesn't affect the users that >>>> do configure a label using the device-tree or the ones that are not >>>> using the labels at all. >>>> >>>> Use the device-tree node name as a default value for `label` property, >>>> in case there isn't one configured through device-tree. >>> >>> Interesting idea. However a few concerns come to mind. >>> 1) If we start having a default for this, then it will get used as ABI >>> and if a label is applied later to the DT then we will end up breaking >>> userspace scripts. >> >> When a label is explicitly configured means that the userspace expects >> to have that value available. Therefore, I don't see this as ABI change, >> given that this affects the property label content and not for example >> the property name. > > The potential issue is that with this userspace code may rely on the common > option (matches device tree node name) and then get confused on it changing. > > If it wasn't there previously and appears (which is what happens when > a label is added currently) userspace is unlikely to have in some fashion > depended on it not being there... > If they rely on missing the label, it's true this will break that. > If someone modifies an existing label they can reasonably expect to break > compatibility because they made something 'go away'. > >> >>> 2) If we do this it should be firmware agnostics (we need to fix >>> the existing code to be such as well). >> >> Not sure I understand this. If you could explain a bit more I would >> really appriciate. > > Typo in that didn't help. (agnostic). Anyhow, basically it has to work > for ACPI as well. > >> >>> 3) Is the node name always unique (think multiple accelerometers on >>> different i2c masters)? >>> 3) I'm fairly sure this information is readily available anyway. >>> either via the of_node link for the iio\:deviceX >>> So why not have your usespace use that instead of label? >>> I'm not a fan of duplicating information that is readily available >>> anyway - be it as name and reg in the of_node directory. >>> >> >> The node name supposed to be unique AFAIK and you're right it is >> available already in the userspace. > > It's not unique. As per https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Felinux.org%2FDevice_Tree_Usage&data=04%7C01%7Cnandor.han%40vaisala.com%7Cdbfd5d178b484f5258fe08d9f6216e65%7C6d7393e041f54c2e9b124c2be5da5c57%7C0%7C0%7C637811445712047047%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=0y2Vx5Cqso5iYAvA58tXnfzI%2BJ9oUbhvDIIkC8XP6Ik%3D&reserved=0, > "sibling nodes are expected to be unique". If you have multiple > i2c masters and the same device under each of them with the > same i2c address they are not siblings (different parents) and > will have the same node name. > Thanks for the info. Keep that in mind > >> My point with this patch is to provide a default value for the label >> content and I'm open to suggestions related to content. The of_node name >> was something that I thought that is unique and easy to use, but if >> somebody has better suggestions I'm really open to these. > > I don't see why we want a default label. If it's not provided it's > not there (no file) and userspace can go use something else for > it's unique naming. Note that for older kernels they need to do > that anyway because label never existed. So userspace will need > to work with possibility of it being absent. As userspace is > going to do that today, why add another option so we have: > > 1) No label attribute. > 2) Label attribute == node name > 3) Label attribute something else > > vs having to handle 2 cases. > > 1) No label attribute > 2) Label attribute present. > > So adding a default makes userspace code more complex, not less. > Thanks for the explanation and review. Given the above comments I guess we will drop this idea. Thanks and Regards, Nandor
Powered by blists - more mailing lists