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