linux-kernel - Re: [PATCH v4 14/14] of: Improve __of_add_property

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: <680a0b08-b085-41aa-a0e3-f36c153c91f6@icloud.com>
Date: Sat, 11 Jan 2025 20:44:34 +0800
From: Zijun Hu <zijun_hu@...oud.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Saravana Kannan <saravanak@...gle.com>,
 Maxime Ripard <mripard@...nel.org>, Robin Murphy <robin.murphy@....com>,
 Grant Likely <grant.likely@...retlab.ca>, Marc Zyngier <maz@...nel.org>,
 Andreas Herrmann <andreas.herrmann@...xeda.com>,
 Marek Szyprowski <m.szyprowski@...sung.com>,
 Catalin Marinas <catalin.marinas@....com>, Mike Rapoport <rppt@...nel.org>,
 Oreoluwa Babatunde <quic_obabatun@...cinc.com>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Zijun Hu <quic_zijuhu@...cinc.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Kees Cook <kees@...nel.org>
Subject: Re: [PATCH v4 14/14] of: Improve __of_add_property_sysfs()
 readability

On 2025/1/11 17:17, Krzysztof Kozlowski wrote:
> On Sat, Jan 11, 2025 at 07:48:48AM +0800, Zijun Hu wrote:
>> On 2025/1/11 04:41, Rob Herring wrote:
>>> On Thu, Jan 09, 2025 at 09:27:05PM +0800, Zijun Hu wrote:
>>>> From: Zijun Hu <quic_zijuhu@...cinc.com>
>>>>
>>>> __of_add_property_sysfs() hard codes string "security-" length as 9, but
>>>> that is not obvious for readers.
>>>>
>>>> Improve its readability by using strlen().
>>>
>>> Does the compiler optimize the strlen call away? Maybe, maybe not. If 
>>> not, that's N calls to strlen() where N is the number of properties in 
>>> your DT. That's in the 1000s easily.
>>>
>>> Do you really want to go test enough compiler versions we support to 
>>> feel confident this is optimized away. I don't.
>>>
>>
>> i understand your concern about performance.
>> perhaps, such impact about performance may be ignored for linux OS.
>>
>> what about below solution ?
>>
>> const char security_prefix[] = "security-";
>> use 'sizeof(security_prefix) - 1' for the length of string.
> 
> Code is still not equivalent - just de-assemble it and you will see
> some overhead.
> 

Thank you Krzysztof Kozlowski for code review

the overhead may be that use of stack is increased a bit.
it may not almost impact performance since 'sizeof(security_prefix) - 1'
may be still a compile time constant.

> Maybe just introduce builtin_strlen() to string.h and use such? It would
> be the pretty obvious code.
> 

it is a good idea to introduce API for this common issue, it seems the
main strncmp() usages in current kernel tree is to check if a string
begin with the other string a shown below:

// "s2_string" is wrote twice, and also impact performance
strncmp(s1, "s2_string", strlen("s2_string"))

// it is not obvious for readers that "s2_string" length is 9
strncmp(s1, "s2_string", 9)

//"s2_string" is wrote twice, same as strcmp(), may be wrong
strncmp(s1, "s2_string", sizeof("s2_string"))

//"s2_string" is wrote twice
strncmp(s1, "s2_string", sizeof("s2_string") - 1)

what about a new API such as ?

// check if @s1 begins with @s2
bool str_begin_with(const char *cs, const char *ct)
{
	unsigned char c1, c2;
	
	if (!cs || !ct)
		return false;
	
	do {
		c1 = *cs++;
		c2 = *ct++;
		
		if (c1 != c2)
			return c2 == '\0';
	} while (c1 != '\0');
	
	return true;
}

> 
> Best regards,
> Krzysztof
>