linux-kernel - Re: [EXT] Re: [PATCH v12 4/5] firmware: imx: add driver for NXP EdgeLock Enclave

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] [day] [month] [year] [list]

Message-ID: <d79c4d59-2186-4846-a202-aeff6d14d0c1@kernel.org>
Date: Wed, 22 Jan 2025 13:57:52 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Pankaj Gupta <pankaj.gupta@....com>, Jonathan Corbet <corbet@....net>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
 Sascha Hauer <s.hauer@...gutronix.de>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>
Cc: "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
 "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [EXT] Re: [PATCH v12 4/5] firmware: imx: add driver for NXP
 EdgeLock Enclave

On 22/01/2025 13:55, Pankaj Gupta wrote:
>>
>>> NAK. Node can be called firmware and your entire driver collapes.
>> The macro is updated to verify the correct-ness of node-name.
> 
>> NAK, do you understand the term? I provided the reasons for NAK.
> Your suggestion is to enable the driver for all allowed node names.
> 1. firmware
> 2. secure
> 3. secure-enclave
> 
> Understood, will allow these names for the driver.

No, you just cannot have such checks.

> 
>>
>> +               (!memcmp(dev_of_node->full_name, NODE_NAME,
>> strlen(NODE_NAME)) ?\ ((strlen(dev_of_node->full_name) >
>> strlen(NODE_NAME)) ?\ GET_ASCII_TO_U8((strlen(dev_of_node-
>>> full_name) - strlen(NODE_NAME)),\ dev_of_node-
>>> full_name[strlen(NODE_NAME) + 1], \ -
>> dev_of_node->full_name[strlen(NODE_NAME) + 2]) : 0)
>> +                               dev_of_node-
>>> full_name[strlen(NODE_NAME) + 2]) : 0) : -EINVAL)
>>
>>>>> +     info_list = device_get_match_data(dev); +     if (idx >>=
>>>>> info_list->>num_mu) { +             dev_err(dev,
>>>>> +                     "Incorrect node name :%s\n",
>>>>> +                     dev->>of_node->>full_name);
>>>
>>>> Nope. "firmware" or "secure" are correct node names.
>>> New check is added to validate the correctness of the node name for
>>> this driver. Replaced the message of " Incorrect node name..", with
>>> the help message.
> 
>> You did not resolve the NAK.
>> 1. You cannot reject correct names.
> Ok, Understood.
>> 2. You cannot add undocumented ABI. You could try to document it, but it will 
>> not solve the first problem.
> Ok. Will not add the ABI document.

Hm? I said you cannot add ABI without documentation and you say you will
not add the ABI?

Well, I NAK-ed this patch and consider all future versions having
anything close to this NAKed as well.



Best regards,
Krzysztof