On Fri, Aug 24, 2018 at 05:57:33PM +0200, Greg Kurz wrote:
> Since commit 32b9c6130762 "Preserve datatype markers when emitting dts
> format", we no longer try to guess the value type. Instead, we reuse
> the type of the datatype markers when they are present, if the type
> is either TYPE_UINT* or TYPE_STRING.
>
> This causes 'dtc -I fs' to crash:
>
> Starting program: /root/dtc -q -f -O dts -I fs /proc/device-tree
> /dts-v1/;
>
> / {
>
> Program received signal SIGSEGV, Segmentation fault.
> __strlen_power8 () at ../sysdeps/powerpc/powerpc64/power8/strlen.S:47
> 47 ld r12,0(r4) /* Load doubleword from memory. */
> (gdb) bt
> #0 __strlen_power8 () at ../sysdeps/powerpc/powerpc64/power8/strlen.S:47
> #1 0x00007ffff7de3d10 in __GI__IO_fputs (str=<optimized out>,
> fp=<optimized out>) at iofputs.c:33
> #2 0x000000001000c7a0 in write_propval (prop=0x100525e0,
> f=0x7ffff7f718a0 <_IO_2_1_stdout_>) at treesource.c:245
>
> The offending line is:
>
> fprintf(f, "%s", delim_start[emit_type]);
>
> where emit_type is TYPE_BLOB and:
>
> static const char *delim_start[] = {
> [TYPE_UINT8] = "[",
> [TYPE_UINT16] = "/bits/ 16 <",
> [TYPE_UINT32] = "<",
> [TYPE_UINT64] = "/bits/ 64 <",
> [TYPE_STRING] = "",
> };
>
> /* Data blobs */
> enum markertype {
> TYPE_NONE,
> REF_PHANDLE,
> REF_PATH,
> LABEL,
> TYPE_UINT8,
> TYPE_UINT16,
> TYPE_UINT32,
> TYPE_UINT64,
> TYPE_BLOB,
> TYPE_STRING,
> };
>
> Because TYPE_BLOB < TYPE_STRING and delim_start[] is a static array,
> delim_start[emit_type] is 0x0. The glibc usually prints out "(null)"
> when one passes 0x0 to %s, but it seems to call fputs() internally if
> the format is exactly "%s", hence the crash.
>
> This could be fixed by adding TYPE_BLOB as well to delim_start[] and
> delim_end[], but then we would face an annoying situation where all
> properties would be printed as a sequence of bytes.
>
> So, instead, when a TYPE_BLOB marker is present, let's try to guess the
> value type and update the marker accordingly. This ensures that emit_type
> points to a valid entry in delim_start[] and delim_end[], thus fixing the
> crash. And this gives a chance to choose a prettier format than a sequence
> of bytes when possible.
>
> While here, let's add an assertion to make it clear which types are
> allowed.
Hrm.. so.. i see the problem.
But is TYPE_BLOB actually worth saving? It basically just says the
data was read in from a file which tells us.. nothing, really.
Seems to me data_copy_file() should just set TYPE_NONE, since it can't
know the type of the data within.
>
> Fixes: 32b9c61307629ac76c6ac0bead6f926d579b3d2c
> Signed-off-by: Greg Kurz <groug@xxxxxxxx>
> ---
> treesource.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/treesource.c b/treesource.c
> index f99544d72344..daf88ed3ce2a 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -228,6 +228,16 @@ static void write_propval(FILE *f, struct property *prop)
> if (m->type < TYPE_UINT8)
> continue;
>
> + if (m->type == TYPE_BLOB)
> + m->type = guess_value_type(prop);
> +
> + /* emit_type MUST have a valid entry in the delimiter arrays */
> + assert(m->type == TYPE_UINT8 ||
> + m->type == TYPE_UINT16 ||
> + m->type == TYPE_UINT32 ||
> + m->type == TYPE_UINT64 ||
> + m->type == TYPE_STRING);
> +
> chunk_len = type_marker_length(m);
> if (!chunk_len)
> chunk_len = len - m->offset;
>
--
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
