linux-kernel - Re: [PATCH] of: add pre-operation notifications

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: <CAL_Jsq+YF-+frAJANSSEO60-O6dmavsE4PJ69VMwSjPAm0UJUQ@mail.gmail.com>
Date:	Wed, 24 Feb 2016 21:41:34 -0600
From:	Rob Herring <robh+dt@...nel.org>
To:	Alan Tull <atull@...nsource.altera.com>
Cc:	Pantelis Antoniou <pantelis.antoniou@...sulko.com>,
	Frank Rowand <frowand.list@...il.com>,
	Grant Likely <grant.likely@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Moritz Fischer <moritz.fischer@...us.com>,
	Pantelis Antoniou <pantelis.antoniou@...il.com>,
	Alan Tull <delicious.quinoa@...il.com>,
	Dinh Nguyen <dinguyen@...nsource.altera.com>
Subject: Re: [PATCH] of: add pre-operation notifications

On Wed, Feb 24, 2016 at 4:28 PM, Alan Tull <atull@...nsource.altera.com> wrote:
> Add pre-apply and pre-remove notifications.
>
> For pre-apply notifications that result from creating an overlay,
> include a device node to the overlay fragment in of_reconfig_data.
>
> If a pre-apply notifier return error, reject the changeset.

A couple of high level comments.

>
> Signed-off-by: Alan Tull <atull@...nsource.altera.com>
> ---
>  drivers/of/base.c    |   20 +++++++++++++
>  drivers/of/dynamic.c |   79 +++++++++++++++++++++++++++++++++++++++++++++-----
>  drivers/of/overlay.c |   46 +++++++++++++++++++++++++----
>  include/linux/of.h   |    7 +++++
>  4 files changed, 138 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 017dd94..6d170e0 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1719,6 +1719,12 @@ int of_add_property(struct device_node *np, struct property *prop)
>
>         mutex_lock(&of_mutex);
>
> +       rc = of_property_notify(OF_RECONFIG_PRE_ADD_PROPERTY, np, prop, NULL);

Do we really need these for properties too? I thought nodes would be enough.

[...]

> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 8225081..9d0c0d9 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -53,13 +53,30 @@ struct of_overlay {
>         struct of_changeset cset;
>  };
>
> +static int of_overlay_notify(unsigned long action, struct device_node *np,
> +                            struct property *prop, struct property *old_prop,
> +                            struct device_node *overlay)
> +{
> +       struct of_reconfig_data rd;
> +
> +       memset(&rd, 0, sizeof(rd));
> +       rd.dn = np;
> +       rd.prop = prop;
> +       rd.old_prop = old_prop;
> +       rd.overlay = overlay;
> +       return of_reconfig_notify(action, &rd);
> +}
> +
>  static int of_overlay_apply_one(struct of_overlay *ov,
> -               struct device_node *target, const struct device_node *overlay);
> +               struct device_node *target, struct device_node *overlay);
>
>  static int of_overlay_apply_single_property(struct of_overlay *ov,
> -               struct device_node *target, struct property *prop)
> +               struct device_node *target, struct property *prop,
> +               struct device_node *overlay)
>  {
>         struct property *propn, *tprop;
> +       unsigned long action;
> +       int ret;
>
>         /* NOTE: Multiple changes of single properties not supported */
>         tprop = of_find_property(target, prop->name, NULL);
> @@ -74,6 +91,15 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
>         if (propn == NULL)
>                 return -ENOMEM;
>
> +       if (!tprop)
> +               action = OF_RECONFIG_PRE_ADD_PROPERTY;
> +       else
> +               action = OF_RECONFIG_PRE_UPDATE_PROPERTY;
> +
> +       ret = of_overlay_notify(action, target, propn, tprop, overlay);
> +       if (ret)
> +               return ret;
> +
I don't understand why the notifier is in the overlay code. All the
notifiers should be contained within the changeset code. Or if we do
need changeset and overlay notifiers, they should be distinct.

>         /* not found? add */
>         if (tprop == NULL)
>                 return of_changeset_add_property(&ov->cset, target, propn);