linux-kernel - Re: [PATCH] OF: base: match each node compatible against all given matches first

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: <529C94B2.3040408@gmail.com>
Date:	Mon, 02 Dec 2013 15:09:54 +0100
From:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
To:	Rob Herring <robherring2@...il.com>,
	Thierry Reding <thierry.reding@...il.com>
CC:	devicetree@...r.kernel.org, Russell King <linux@....linux.org.uk>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	linux-kernel@...r.kernel.org,
	Grant Likely <grant.likely@...aro.org>,
	linux-arm-kernel@...ts.infradead.org, Meelis Roos <mroos@...ux.ee>,
	Marc Kleine-Budde <mkl@...gutronix.de>,
	Scott Wood <scottwood@...escale.com>
Subject: Re: [PATCH] OF: base: match each node compatible against all given
 matches first

On 12/02/13 15:00, Rob Herring wrote:
> On 12/02/2013 06:03 AM, Thierry Reding wrote:
>> On Thu, Nov 28, 2013 at 07:36:25PM +0100, Sebastian Hesselbarth
>> wrote:
>>> Currently, of_match_node compares each given match against all
>>> node's compatible strings with of_device_is_compatible.
>>>
>>> To achieve multiple compatible strings per node with ordering
>>> from specific to generic, this requires given matches to be
>>> ordered from specific to generic. For most of the drivers this
>>> is not true and also an alphabetical ordering is more sane
>>> there.
>>>
>>> Therefore, this patch modifies of_match_node to match each of
>>> the node's compatible strings against all given matches first,
>>> before checking the next compatible string. This implies that
>>> node's compatibles are ordered from specific to generic while
>>> given matches can be in any order.
>>>
>>> Signed-off-by: Sebastian Hesselbarth
[...]
>> That said, I think you might actually have nailed it with this
>> patch. From what I remember all earlier attempt failed because
>> they didn't match all compatible/name/type combinations properly.
>> I'm adding a few people on Cc who were involved with the other
>> patches, perhaps they can give your patch a spin and see if it
>> fixes things for them.
>
> I agree and the change here is simple enough I think the chances of a
> regression are low. But there is one issue...
>
>>
>> Thierry
>>
>> [0]: https://lkml.org/lkml/2013/10/3/585
>>
>>> diff --git a/drivers/of/base.c b/drivers/of/base.c index
>>> 7d4c70f..183a9c7 100644 --- a/drivers/of/base.c +++
>>> b/drivers/of/base.c @@ -713,23 +713,37 @@ static const struct
>>> of_device_id *__of_match_node(const struct of_device_id *matches,
>>> const struct device_node *node) { +	const char *cp; + int cplen,
>>> l; + if (!matches) return NULL;
>>>
>>> -	while (matches->name[0] || matches->type[0] ||
>>> matches->compatible[0]) { -		int match = 1; -		if
>>> (matches->name[0]) -			match &= node->name -				&&
>>> !strcmp(matches->name, node->name); -		if (matches->type[0]) -
>>> match &= node->type -				&& !strcmp(matches->type, node->type);
>>> - if (matches->compatible[0]) -			match &=
>>> __of_device_is_compatible(node, -							   matches->compatible);
>>>   -		if (match) -			return matches; -		matches++; +	cp =
>>> __of_get_property(node, "compatible", &cplen); +	if (!cp) +
>>> return NULL;
>
> No compatible property and matching on name and/or type only is valid.
> Remove the if here and changing the following should work:
>
>>> + +	while (cplen > 0) {
>
> do {
>
>>> +		const struct of_device_id *m = matches; + +		while
>>> (m->name[0] || m->type[0] || m->compatible[0]) { +			int match =
>>> 1; +			if (m->name[0]) +				match &= node->name +					&&
>>> !strcmp(m->name, node->name); +			if (m->type[0]) +				match &=
>>> node->type + && !strcmp(m->type, node->type); +			if
>>> (m->compatible[0])
>
> if (cp && m->compatible[0])
>
>>> +				match &= !of_compat_cmp(cp, m->compatible, +
>>> strlen(m->compatible)); +			if (match) +				return m; +			m++; +
>>> } +		l = strlen(cp) + 1; +		cp += l; +		cplen -= l; }
>
>
> } while (cp && (cplen > 0));

Rob,

although quoting somehow got messed up, I see the point. The pointer
arith right above loop end condition needs to be skipped too if !cp.

I will rework the loop to allow name/type matching with cp == NULL
and resend in a day or two.

Sebastian

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/