| 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
| ||
|
Message-Id: <2B34FBA8-A9DB-4DB5-BF95-4FE752EAF128@antoniou-consulting.com>
Date: Tue, 10 May 2016 16:56:43 +0300
From: Pantelis Antoniou <panto@...oniou-consulting.com>
To: Rob Herring <robherring2@...il.com>
Cc: Frank Rowand <frowand.list@...il.com>,
Matt Porter <mporter@...sulko.com>,
Grant Likely <grant.likely@...retlab.ca>,
Koen Kooi <koen@...inion.thruhere.net>,
Guenter Roeck <linux@...ck-us.net>,
Marek Vasut <marex@...x.de>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] of: overlay: Pick up label symbols from overlays.
Hi Rob,
> On May 10, 2016, at 00:47 , Rob Herring <robherring2@...il.com> wrote:
>
> On Mon, May 9, 2016 at 1:11 PM, Pantelis Antoniou
> <pantelis.antoniou@...sulko.com> wrote:
>> Insert overlay symbols to the base tree when applied.
>> This makes it possible to apply an overlay that references a label
>> that a previously inserted overlay had.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@...sulko.com>
>> Tested-by: Geert Uytterhoeven <geert+renesas@...der.be>
>>
>> ---
>> drivers/of/overlay.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 102 insertions(+)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index a274d65..a7956a2 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -514,6 +514,101 @@ static int of_free_overlay_info(struct of_overlay *ov)
>> return 0;
>> }
>>
>> +static int of_overlay_add_symbols(
>> + struct device_node *tree,
>> + struct of_overlay *ov)
>> +{
>> + struct of_overlay_info *ovinfo;
>> + struct device_node *root_sym = NULL;
>> + struct device_node *child = NULL;
>> + struct property *prop;
>> + const char *path, *s;
>> + char *new_path;
>> + int i, len, err;
>> +
>> + /* this may fail (if no fixups are required) */
>> + root_sym = of_find_node_by_path("/__symbols__");
>> +
>> + /* do nothing if no root symbols */
>> + if (!root_sym)
>> + return 0;
>> +
>> + /* locate the symbols & fixups nodes on resolve */
>> + for_each_child_of_node(tree, child) {
>> + if (of_node_cmp(child->name, "__symbols__") == 0)
>> + goto found;
>
> Doesn't of_get_child_by_name work here?
>
No, we’re holding of_mutex.
>> + }
>> + /* no symbols, no problem */
>> + of_node_put(root_sym);
>> + return 0;
>> +
>> +found:
>
> As Marek said, get rid of the goto.
>
>> + err = -EINVAL;
>> + for_each_property_of_node(child, prop) {
>> +
>> + /* skip properties added automatically */
>> + if (of_prop_cmp(prop->name, "name") == 0)
>> + continue;
>> +
>> + err = of_property_read_string(child,
>> + prop->name, &path);
>> + if (err != 0) {
>> + pr_err("%s: Could not find symbol '%s'\n",
>> + __func__, prop->name);
>
> Can you introduce and use a common pr_fmt prefix here and elsewhere
> (just what you are adding).
OK
>
>> + continue;
>> + }
>> +
>> +
>
> Extra blank line.
>
>> + /* now find fragment index */
>> + s = path;
>> +
>> + /* compare paths to find fragment index */
>> + ovinfo = NULL;
>> + len = -1;
>
> Move these into the for init.
>
>> + for (i = 0; i < ov->count; i++) {
>> + ovinfo = &ov->ovinfo_tab[i];
>> +
>> + pr_debug("%s: #%d: overlay->name=%s target->name=%s\n",
>> + __func__, i, ovinfo->overlay->full_name,
>> + ovinfo->target->full_name);
>> +
>> + len = strlen(ovinfo->overlay->full_name);
>> + if (strncasecmp(path, ovinfo->overlay->full_name,
>> + len) == 0 && path[len] == '/')
>> + break;
>> + }
>> +
>> + if (i >= ov->count)
>> + continue;
>> +
>> + pr_debug("%s: found target at #%d\n", __func__, i);
>> + new_path = kasprintf(GFP_KERNEL, "%s%s",
>> + ovinfo->target->full_name,
>> + path + len);
>> + if (!new_path) {
>> + pr_err("%s: Failed to allocate propname for \"%s\"\n",
>> + __func__, prop->name);
>> + continue;
>
> If this fails, is there much point in continuing?
>
No
>> + }
>> +
>> + err = of_changeset_add_property_string(&ov->cset, root_sym,
>> + prop->name, new_path);
>> +
>> + /* free always */
>> + kfree(new_path);
>> +
>> + if (err) {
>
> No need for brackets.
>
>> + pr_err("%s: Failed to add property for \"%s\"\n",
>> + __func__, prop->name);
>> + }
>> + }
>> +
>> + of_node_put(child);
>> + of_node_put(root_sym);
>> +
>> + return 0;
>> +}
>> +
>> static LIST_HEAD(ov_list);
>> static DEFINE_IDR(ov_idr);
>>
>> @@ -642,6 +737,13 @@ static int __of_overlay_create(struct device_node *tree,
>> goto err_abort_trans;
>> }
>>
>> + err = of_overlay_add_symbols(tree, ov);
>> + if (err) {
>> + pr_err("%s: of_overlay_add_symbols() failed for tree@%s\n",
>> + __func__, tree->full_name);
>> + goto err_abort_trans;
>> + }
>> +
>> /* apply the changeset */
>> err = __of_changeset_apply(&ov->cset);
>> if (err) {
>> --
>> 1.7.12
>>
Regards
— Pantelis
Powered by blists - more mailing lists