On Thu, May 09, 2019 at 07:41:20PM +1000, Nicholas Piggin wrote:
> If fdt_add_property or fdt_property_placeholder fail after allocating
> a string for the name, they return without freeing that string. This
> does not change the structure of the tree, but in very specific cases
> it could lead to undesirable space consumption.
>
> Fix this by rolling back the string allocation in this situation.
>
> Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx>
Applied, but there's a small followup cleanup I'd like to see..
> ---
> libfdt/fdt_rw.c | 22 ++++++++--
> libfdt/fdt_sw.c | 23 +++++++++--
> tests/.gitignore | 1 +
> tests/Makefile.tests | 2 +-
> tests/run_tests.sh | 2 +
> tests/rw_oom.c | 96 ++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 139 insertions(+), 7 deletions(-)
> create mode 100644 tests/rw_oom.c
>
> diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
> index 2e49855..9e76615 100644
> --- a/libfdt/fdt_rw.c
> +++ b/libfdt/fdt_rw.c
> @@ -136,6 +136,14 @@ static int fdt_splice_struct_(void *fdt, void *p,
> return 0;
> }
>
> +/* Must only be used to roll back in case of error */
> +static void fdt_del_last_string_(void *fdt, const char *s)
> +{
> + int newlen = strlen(s) + 1;
> +
> + fdt_set_size_dt_strings(fdt, fdt_size_dt_strings(fdt) - newlen);
> +}
This is functionally, but not textually equivalent to ...
[snip]
> diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c
> index 9fa4a94..113f28a 100644
> --- a/libfdt/fdt_sw.c
> +++ b/libfdt/fdt_sw.c
> @@ -262,7 +262,16 @@ int fdt_end_node(void *fdt)
> return 0;
> }
>
> -static int fdt_find_add_string_(void *fdt, const char *s)
> +/* Must only be used to roll back in case of error */
> +static void fdt_del_last_string_(void *fdt, const char *s)
> +{
> + int strtabsize = fdt_size_dt_strings(fdt);
> + int len = strlen(s) + 1;
> +
> + fdt_set_size_dt_strings(fdt, strtabsize - len);
> +}
..this, which is kinda ugly.
Better to share a version via libfdt_internal.h, I think.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
