libfdt: Replace deprecated 0-length arrays with proper flexible arrays

[kees]

Replace the 0-length arrays in structures with proper flexible arrays. This will avoid warnings when building under GCC 13 with -fstrict-flex-arrays, which the Linux kernel will be doing soon:

In file included from ../lib/fdt_ro.c:2:
../lib/../scripts/dtc/libfdt/fdt_ro.c: In function 'fdt_get_name': ../lib/../scripts/dtc/libfdt/fdt_ro.c:319:24: warning: 'strrchr' reading 1 or more bytes from a region of size 0 [-Wstringop-overread]
319 | leaf = strrchr(nameptr, '/');
| ^~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Kees Cook keescook@chromium.org

[kees]

It seems like the freebsd tests have been failing for a while, looking at the recent commits.

[dgibson]

Replace the 0-length arrays in structures with proper flexible arrays. This will avoid warnings when building under GCC 13 with -fstrict-flex-arrays, which the Linux kernel will be doing soon:

Hrm, so, using standard constructs is good. It's particularly good for libfdt.h which is supposed to cover the format itself, rather than be bound strictly to libfdt.

I had avoided this, because I was under the impression that flexible array elements counted as length 1, rather than 0 for the purposes of sizeof which will break a number of places in libfdt. However, I've just had another look, and it seems like I was mistaken.

If you can double check that with a definitive source, that would be most helpful.

[a3f]

If you can double check that with a definitive source, that would be most helpful.

Quoting C99 §6.7.2.1 "Structure and union specifiers" ¶16:

As a special case, the last element of a structure with more than one named member may
have an incomplete array type; this is called a flexible array member . In most situations,
the flexible array member is ignored. In particular, the size of the structure is as if the
flexible array member were omitted except that it may have more trailing padding than
the omission would imply.

[dgibson]

Thanks. Applied.

[robherring]

This change is breaking the build of pylibfdt for me:
/home/rob/proj/dtc/./pylibfdt/libfdt_wrap.c: In function ‘_wrap_fdt_node_header_name_set’:
/home/rob/proj/dtc/./pylibfdt/libfdt_wrap.c:4350:18: error: cast specifies array type
4350 | arg1->name = (char [])(char *)memcpy(malloc((size)*sizeof(char)), (const char )(arg2), sizeof(char)(size));
| ^
/home/rob/proj/dtc/./pylibfdt/libfdt_wrap.c:4350:16: error: invalid use of flexible array member
4350 | arg1->name = (char [])(char *)memcpy(malloc((size)*sizeof(char)), (const char )(arg2), sizeof(char)(size));
| ^
/home/rob/proj/dtc/./pylibfdt/libfdt_wrap.c:4352:16: error: invalid use of flexible array member
4352 | arg1->name = 0;
| ^
/home/rob/proj/dtc/./pylibfdt/libfdt_wrap.c: In function ‘_wrap_fdt_property_data_set’:
/home/rob/proj/dtc/./pylibfdt/libfdt_wrap.c:4613:18: error: cast specifies array type
4613 | arg1->data = (char [])(char *)memcpy(malloc((size)*sizeof(char)), (const char )(arg2), sizeof(char)(size));
| ^
/home/rob/proj/dtc/./pylibfdt/libfdt_wrap.c:4613:16: error: invalid use of flexible array member
4613 | arg1->data = (char [])(char *)memcpy(malloc((size)*sizeof(char)), (const char )(arg2), sizeof(char)(size));
| ^
/home/rob/proj/dtc/./pylibfdt/libfdt_wrap.c:4615:16: error: invalid use of flexible array member
4615 | arg1->data = 0;
| ^

I'm using gcc 12.2.0.

[kees]

Uhm, is that some kind of SWIG binding? We've seen issues with non-C scrapers falling over in the face of flex arrays before: https://www.spinics.net/lists/fedora-devel/msg297996.html

[robherring]

Yes, it's SWIG. Paging @sjg20 ...

Looks like the distro fixes are either reverting the change or avoiding the struct for SWIG. Will see if we can do the latter.

[sjg20]

Yes it breaks for me too...will take a quick look

[sjg20]

https://stackoverflow.com/questions/29781102/swig-handling-tlvzero-length-array

Hmm I think reverting is the best..

Kees are you able to figure out the swig stuff and resubmit?

[sjg20]

(and how did this pass for you?)

[robherring]

I have a fix:
diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
index f9f7e7e66d13..efbdee8c22d8 100644
--- a/pylibfdt/libfdt.i
+++ b/pylibfdt/libfdt.i
@@ -1036,6 +1036,9 @@ class NodeAdder():

%rename(fdt_property) fdt_property_func;

+%ignore fdt_property::data;
+%ignore fdt_node_header::name;
+
/*

• fdt32_t is a big-endian 32-bit value defined to uint32_t in libfdt_env.h
• so use the same type here.

[sjg20]

Seems right. We don't need to set these through the swig interface anyway

[robherring]

Patch on the list.

Note that it passed CI tests, so I guess Python is skipped for those.

[sjg20]

Funny that that one went to spam

[dgibson]

@robherring @sjg20 the reason I didn't detect this breakage is that the build of pylibfdt has been broken for me for months. It's cleearly not broken for everyone, so it must be something specific to my build environment (Fedora 37), but I don't know how to fix it.

In any case I've now applied the fix.