Re: [PATCH] Try to guess type of TYPE_BLOB when emitting dts format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



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


[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux