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]
Date:	Tue, 30 Jun 2015 18:21:31 +0100
From:	Grant Likely <grant.likely@...aro.org>
To:	Geert Uytterhoeven <geert+renesas@...der.be>,
	Pantelis Antoniou <pantelis.antoniou@...sulko.com>,
	Rob Herring <robh+dt@...nel.org>
Cc:	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Geert Uytterhoeven <geert+renesas@...der.be>
Subject: Re: [PATCH/RFC 3/3] of/dynamic: Update list of aliases on aliases
 changes

On Tue, 30 Jun 2015 16:51:16 +0200
, Geert Uytterhoeven <geert+renesas@...der.be>
 wrote:
> Currently the list of aliases is not updated when an overlay that
> modifies /aliases is added or removed. This breaks drivers (e.g. serial)
> that rely on of_alias_get_id().
> 
> Update the list of aliases when a property of the /aliases node is
> added, removed, or updated.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
> ---
>   - Should the OF core handle this itself, by registering a notifier
>     using of_reconfig_notifier_register()?

Yes. Let's not add new hooks.

>   - Adding or destroying the /aliases node itself should be handled,
>     too.

Yes

>   - Is it safe to deallocate struct alias_prop using kfree()? It may
>     have been allocated using early_init_dt_alloc_memory_arch() /
>     memblock_alloc(). What's the alternative? Leaking memory?

Properties are not refcounted, so yes we leak memory. The memory remains
owned by the aliases node, but because the aliases node is never freed,
neither are any of the properties. Solving this isn't easy because it
would require adding refcounting *everywhere* that properties are
accessed. I think we have to just live with it until someone clever can
some up with a solution.

g.

> ---
>  drivers/of/dynamic.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 1901f8870591fe30..65dfb26f879c3a9a 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -502,6 +502,16 @@ static void __of_changeset_entry_invert(struct of_changeset_entry *ce,
>  	}
>  }
>  
> +static void *alias_alloc(u64 size, u64 align)
> +{
> +	return kzalloc(size, GFP_KERNEL);
> +}
> +
> +static void alias_free(void *p)
> +{
> +	kfree(p);
> +}
> +
>  static void __of_changeset_entry_notify(struct of_changeset_entry *ce, bool revert)
>  {
>  	struct of_reconfig_data rd;
> @@ -513,6 +523,20 @@ static void __of_changeset_entry_notify(struct of_changeset_entry *ce, bool reve
>  		ce = &ce_inverted;
>  	}
>  
> +	if (ce->np == of_aliases) {
> +		switch (ce->action) {
> +		case OF_RECONFIG_ADD_PROPERTY:
> +			of_alias_create(ce->prop, alias_alloc);
> +			break;
> +		case OF_RECONFIG_REMOVE_PROPERTY:
> +			of_alias_destroy(ce->prop->name, alias_free);
> +			break;
> +		case OF_RECONFIG_UPDATE_PROPERTY:
> +			of_alias_destroy(ce->old_prop->name, alias_free);
> +			of_alias_create(ce->prop, alias_alloc);
> +			break;
> +		}
> +	}
>  	switch (ce->action) {
>  	case OF_RECONFIG_ATTACH_NODE:
>  	case OF_RECONFIG_DETACH_NODE:
> -- 
> 1.9.1
> 

--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ