On 11/28/17 14:26, Alan Tull wrote:
> On Tue, Nov 28, 2017 at 9:15 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
>> On Mon, Nov 27, 2017 at 02:58:03PM -0600, Alan Tull wrote:
>>> Add simple whitelist. When an overlay is submitted, if any target in
>>> the overlay is not in the whitelist, the overlay is rejected. Drivers
>>> that support dynamic configuration can register their device node with:
>>>
>>> int of_add_whitelist_node(struct device_node *np)
>>>
>>> and remove themselves with:
>>>
>>> void of_remove_whitelist_node(struct device_node *np)
>>
>> I think these should be named for what they do, not how it is
>> implemented.
>
> Sure, such as of_node_overlay_enable and of_node_overlay_disable?
of_allow_overlay_on_node(), of_disallow_overlay_on_node()?
>
>>
>>>
>>> Signed-off-by: Alan Tull <atull@xxxxxxxxxx>
>>> ---
>>> drivers/of/overlay.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/of.h | 12 +++++++++
>>> 2 files changed, 85 insertions(+)
>>>
>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>>> index c150abb..5f952a1 100644
>>> --- a/drivers/of/overlay.c
>>> +++ b/drivers/of/overlay.c
>>> @@ -21,6 +21,7 @@
>>> #include <linux/slab.h>
>>> #include <linux/err.h>
>>> #include <linux/idr.h>
>>> +#include <linux/spinlock.h>
>>>
>>> #include "of_private.h"
>>>
>>> @@ -646,6 +647,74 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
>>> kfree(ovcs);
>>> }
>>>
>>> +/* lock for adding/removing device nodes to the whitelist */
>>> +static spinlock_t whitelist_lock;
>>> +
>>> +static struct list_head whitelist_list = LIST_HEAD_INIT(whitelist_list);
>>> +
>>> +struct dt_overlay_whitelist {
>>> + struct device_node *np;
>>> + struct list_head node;
>>> +};
>>
>> Can't we just add a flags bit in device_node.flags? That would be much
>> simpler.
>
> Yes, much simpler. Such as:
>
> #define OF_OVERLAY_ENABLED 5 /* allow DT overlay targeting this node */
>
>>
>>> +
>>> +int of_add_whitelist_node(struct device_node *np)
>>> +{
>>> + unsigned long flags;
>>> + struct dt_overlay_whitelist *wln;
>>> +
>>> + wln = kzalloc(sizeof(*wln), GFP_KERNEL);
>>> + if (!wln)
>>> + return -ENOMEM;
>>> +
>>> + wln->np = np;
>>> +
>>> + spin_lock_irqsave(&whitelist_lock, flags);
>>> + list_add(&wln->node, &whitelist_list);
>>> + spin_unlock_irqrestore(&whitelist_lock, flags);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(of_add_whitelist_node);
>>> +
>>> +void of_remove_whitelist_node(struct device_node *np)
>>> +{
>>> + struct dt_overlay_whitelist *wln;
>>> + unsigned long flags;
>>> +
>>> + list_for_each_entry(wln, &whitelist_list, node) {
>>> + if (np == wln->np) {
>>> + spin_lock_irqsave(&whitelist_lock, flags);
>>> + list_del(&wln->node);
>>> + spin_unlock_irqrestore(&whitelist_lock, flags);
>>> + kfree(wln);
>>> + return;
>>> + }
>>> + }
>>> +}
>>> +EXPORT_SYMBOL_GPL(of_remove_whitelist_node);
>>> +
>>> +static int of_check_whitelist(struct overlay_changeset *ovcs)
>>> +{
>>> + struct dt_overlay_whitelist *wln;
>>> + struct device_node *target;
>>> + int i;
>>> +
>>> + for (i = 0; i < ovcs->count; i++) {
>>> + target = ovcs->fragments[i].target;
>>> + if (!of_node_cmp(target->name, "__symbols__"))
>>> + continue;
>>> +
>>> + list_for_each_entry(wln, &whitelist_list, node)
>>> + if (target == wln->np)
>>> + break;
>>> +
>>> + if (target != wln->np)
>>> + return -ENODEV;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> /**
>>> * of_overlay_apply() - Create and apply an overlay changeset
>>> * @tree: Expanded overlay device tree
>>> @@ -717,6 +786,10 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
>>> if (ret)
>>> goto err_free_overlay_changeset;
>>>
>>> + ret = of_check_whitelist(ovcs);
>>> + if (ret)
>>> + goto err_free_overlay_changeset;
>>
>> This will break you until the next patch and breaks any other users. I
>> think this is now just the unittest as tilcdc overlay is getting
>> removed.
>>
>> You have to make this chunk the last patch in the series.
>
> I'd rather squash the two patches. In either case, the contents of
> second patch are dependent on stuff in char-misc-testing today, so it
> won't be able to apply yet on linux-next or anywhere else.
>
> Thanks
> Alan
>
>>
>> Rob
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
