The name field of 'struct node' was really always supposed to be a
malloc()ed string, that is owned by the structure. To avoid an extra
strdup() for strings coming up from the lexer, name_node() expects to take
uch an already malloc()ed string, which means it's not correct to pass it
a static string literal.
That's a pretty non-obvious constraint, so a bunch of incorrect uses have
crept in. Really, avoiding the extra dup from the lexer isn't a big enough
benefit for this demonstrably dangerous interface. So change it to do the
xstrdup() itself, removing the burden from callers.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The 'name' field of 'struct node' is supposed to be an (individually)
malloc()ed string. So, when taking a name from a flattened blob we need
to strdup() it.
Currently that happens in flat_read_string() as we take it from the
flattened structure itself. That obscures what's going on because it's
several steps removed from actually inserting it into node->name. It also
means we need an additional strdup() and free() for the case of old dtb
formats where we need to extract just the final path component from the
blob for the name.
While we're scanning the blob, we're doing so read-only, so it's fine to
have pointers into it. Therefore simplify things a bit by delaying the
xstrdup() to the point where we're actually inserting into node->name.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The name field of 'struct property' was really always supposed to be a
malloc()ed string, that is owned by the structure. To avoid an extra
strdup() for strings coming up from the lexer, build_property() and
build_property_delete() expect to take such an already malloc()ed string,
which means it's not correct to pass it a static string literal.
That's a pretty non-obvious constraint, so a bunch of incorrect uses have
crept in. Really, avoiding the extra dup from the lexer isn't a big enough
benefit for this demonstrably dangerous interface. So change it to do the
xstrdup() itself, removing the burden from callers.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Currently if there is a valid 10-bit address the following warning is
always displayed due to the 7-bit check failing due to reg > 0x7f
"I2C address must be less than 7-bits, got "0x800000a6". Set I2C_TEN_BIT_ADDRESS for 10 bit addresses or fix the property"
Fix this issue by checking if a 10-bit address is expected, and is valid in separate if statements.
Fixes: 8259d59f ("checks: Improve i2c reg property checking")
Signed-off-by: Matt Ranostay <matt.ranostay@konsulko.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Because meson always builds out-of-tree we need to reference things in the
original source tree via $SRCDIR from run_tests.sh. We forgot a couple of
cases for the cell overflow tests. Fix them.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add this new warning to the default build flags. It suggests adding
a ((format)) attribute to xavsprintf_append(), so do that.
Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
It's been rather too long since the last release, and quite a lot of
changes have accumulated. Finally get around to rolling a release.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This also enables us to test the -NOSPACE condition by adding a test
setting size_hint=1 so this path is taken.
Message-Id: <20230201181112.1644842-1-luca@z3ntu.xyz>
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
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>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
We always evaluate integer values in cell arrays as 64-bit quantities, then
truncate to the size of the array cells (32-bit by default). However to
detect accidental truncation of meaningful values, we give an error if the
truncated portion isn't either all 0 or all 1 bits. However, this can
still give counterintuitive errors. For if the user is thinking in 2's
complement 32-bit arithmetic (which would be quite natural), then they'd
expect the expression (-0xffffffff-2) to evaluate to -1 (0xffffffff).
However in 64-bit it evaluates to 0xfffffffeffffffff which does truncate
to the expected value but trips this error message.
Because of this reduce the error to only a warnings, with a somewhat more
helpful message.
Fixes: https://github.com/dgibson/dtc/issues/74
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add a new test get_next_tag_invalid_prop_len, which covers
fdt_next_tag(), when it is passed an corrupted blob, with
invalid property len values. The test runs twice, on a blob
in sw and finished state.
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Message-Id: <20221011182611.116011-2-tadeusz.struk@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Since fdt_next_tag() in a public API function all input parameters,
including the fdt blob should not be trusted. It is possible to forge
a blob with invalid property length that will cause integer overflow
during offset calculation. To prevent that, validate the property length
read from the blob before doing calculations.
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Message-Id: <20221005232931.3016047-1-tadeusz.struk@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add a new fdt_get_property_by_offset_w helper function.
It is a wrapper on fdt_get_property_by_offset that returns
a writable pointer to a property at a given offset.
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Message-Id: <20221011182611.116011-1-tadeusz.struk@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Without the change GNU `make-4.4` falls into infinite recursion of trying
to generate %.output files (bison is not passed flags to generate debug
output).
This happens on GNU `make-4.4` only after GNU make change to more eagerly
rebuild all target outputs in multiple targets:
https://savannah.gnu.org/bugs/index.php?63098
The recursion here is the following:
- Makefile depends on *.d files
- *.d files depend on *.c files
- *.c files are generated by bison
- bison is triggered whenever some of it's multiple targets are missing
In our case `%.output` is always missing and bison is always reran.
*.d files are always regenerated on `make` run. And make is always
restarted as *.d files are always regenerated.
The fix removes infeasible `%.output`.
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
Message-Id: <20220925104203.648449-2-slyich@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
make-4.4 became intentionally more eager at rebuilding outdated Makefile
includes. Currently this causes `dtc` to spin infinitely in
parser/dependency loop:
$ make
...
CHK version_gen.h
BISON dtc-parser.tab.h
DEP dtc-lexer.lex.c
DEP dtc-parser.tab.c
CHK version_gen.h
BISON dtc-parser.tab.h
DEP dtc-lexer.lex.c
DEP dtc-parser.tab.c
... # never stops
After the change build eventually fails when gets into this state:
$ make
...
CHK version_gen.h
UPD version_gen.h
DEP util.c
BISON dtc-parser.tab.h
DEP dtc-lexer.lex.c
DEP dtc-parser.tab.c
CHK version_gen.h
BISON dtc-parser.tab.h
DEP dtc-lexer.lex.c
DEP dtc-parser.tab.c
Makefile:394: *** "Make re-executed itself 10 times. Infinite recursion?". Stop.
The actual recursion will be fixed separately.
Signed-off-by: Sergei Trofimovich <slyich@gmail.com>
Message-Id: <20220925104203.648449-1-slyich@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
There is no need to check the VALID_DTB repeatedly, and can be combined
into one if statement.
Signed-off-by: Jia Xianhua <jiaxianhua@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The dtb overlay format only permits (non local) fixups to reference labels,
not paths. That's because the fixup target goes into the property name in
the overlay, and property names aren't permitted to include '/' characters.
Stop erroneously generating such fixups, because we didn't check for this
case.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Return the error code from fdt_get_name() (contained in len when the
result is NULL) instead of masking it with FDT_ERR_BADSTRUCTURE.
Fixes: fda71da26e ("libfdt: Handle failed get_name() on BEGIN_NODE")
Reported-by: Mike McTernan <mikemcternan@google.com>
Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
Message-Id: <20220729130019.804288-1-ptosi@google.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Following 0ee1d479b2 ("Remove Jon Loeliger from maintainers list"),
make the "Submitting Patches" section of the manual.txt consistent with
the README by requesting patches to only be sent to David.
Cc: Jon Loeliger <loeliger@gmail.com>
Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
Message-Id: <20220729131019.806164-1-ptosi@google.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
dtc and libfdt have been using Signed-off-by lines (as used in the Linux
kernel) for some time, like a lot of open source projects. However
Uwe Kleine-König pointed out we never really stated what they mean in our
context.
Add information on what the S-o-b line means in CONTRIBUTING.md - this is
essentially a quote of the same information from the kernel documentation,
with some tweaks to make sense in the new context.
Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
README.md covers both general information for people using and building
the software, and more specific information for people contributing to
either dtc or libfdt. Split out the latter information into its own file
for easier reference.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Let's move vaguely into the twenty-first century by converting our old
plain text README file to Markdown. While we're updating the formatting,
make some small polish changes to the content.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Added "static-build" option in the meson_options.txt.
Setting it to "true" allows static building.
Signed-off-by: Tero Tervala <tero.tervala@unikie.com>
Message-Id: <20220629163557.932298-1-tero.tervala@unikie.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Set STATIC_BUILD=1 environment variable to enable static building
when using makefiles.
Signed-off-by: Tero Tervala <tero.tervala@unikie.com>
Message-Id: <20220629163531.932281-1-tero.tervala@unikie.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Validate the return value of fdt_get_name() as an ill-formed DT, causing
it to fail, could result in fdt_check_full() dereferencing NULL.
fixes: a2def54799 ("libfdt: Check that the root-node name is empty")
Signed-off-by: Pierre-Clément Tosi <ptosi@google.com>
Message-Id: <20220714083848.958492-1-ptosi@google.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
/bin/sh points to dash instead of bash in some linux distros.
One test would fail if dash was used, this fix will allow all tests
to run properly on dash too.
dash built-in printf does not support "\xNN" -hex escape format.
"\NNN" -octal escape format is supported by both bash and dash printf.
Replaced "$(echo "$expect")" because this actually runs /bin/echo
instead of shell internal echo and in some cases causes "\NNN" escapes
to be printed as the actual characters they represent instead of the
escape sequence itself.
Cosmetic quotes added to make printout a bit clearer.
Signed-off-by: Tero Tervala <tero.tervala@unikie.com>
Message-Id: <20220704073722.1075849-1-tero.tervala@unikie.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Will remove one "Strange test result" when running tests with
meson
Signed-off-by: Tero Tervala <tero.tervala@unikie.com>
Message-Id: <20220629163114.932175-1-tero.tervala@unikie.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add a new Python method wrapping fdt_get_path() from the C API.
Also add a test for the new method.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Message-Id: <20220419194537.63170-1-luca@z3ntu.xyz>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
A 'pip install' is silently broken unless the tree is dirty and contains
pylibfdt/libfdt.py. The problem is a known issue[1] with SWIG and
setuptools where the 'build_py' stage needing module.py runs before
the 'build_ext' stage which generates it. The work-around is to override
'build_py' to run 'build_ext' first.
[1] https://stackoverflow.com/questions/50239473/building-a-module-with-setuptools-and-swig
Signed-off-by: Rob Herring <robh@kernel.org>
Message-Id: <20220203180408.611645-2-robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reference via label allows extending nodes with compile-time checking of
whether the node being extended exists. This is useful to catch
renamed/removed nodes after an update of the device trees to be extended.
In absence of labels in the original device trees, new style path
references can be used:
/* upstream device tree */
/ {
leds: some-non-standard-led-controller-name {
led-0 {
default-state = "off";
};
};
};
/* downstream device tree */
&{/some-non-standard-led-controller-name/led-0} {
default-state = "on";
};
This is a common theme within the barebox bootloader[0], which extends the
upstream (Linux) device trees in that manner. The downside is that,
especially for deep nodes, these references can get quite long and tend to
break often due to upstream rework (e.g. rename to adhere to bindings).
Often there is a label a level or two higher that could be used. This
patch allows combining both a label and a new style path reference to
get a compile-time-checked reference, which allows rewriting the
previous downstream device tree snippet to:
&{leds/led-0} {
default-state = "on";
};
This won't be broken when /some-non-standard-led-controller-name is
renamed or moved while keeping the label. And if led-0 is renamed, we
will get the expected compile-time error.
Overlay support is skipped for now as they require special support: The
label and relative path parts need to be resolved at overlay apply-time,
not at compile-time.
[0]: https://www.barebox.org/doc/latest/devicetree/index.html
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
We already have xstrdup, add xstrndup as well to make it
straight-forward to clone part of a string.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
If the corresponding '#xxx-cells' value is much too large, an integer
overflow can prevent the checks in check_property_phandle_args() from
correctly determining that the checked property is too short for the
given cells value. This leads to an infinite loops.
This patch fixes the bug, and adds a testcase for it. Further
information in https://github.com/dgibson/dtc/issues/64
Reported-by: Anciety <anciety@pku.edu.cn>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This is not obvious so add a little note about it.
Signed-off-by: Simon Glass <sjg@chromium.org>
Message-Id: <20211107224346.3181320-2-sjg@chromium.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
When fdt_get_string return null, `namep` is not correctly reset.
From the document of `fdt_getprop_by_offset`, the parameter `namep` will
be always overwritten (that is, it will be overwritten without exception
of error occurance).
As for the caller (like
e097c097fe/native/jni/magiskboot/dtb.cpp (L42)),
the code may be like:
```cpp
size_t size;
const char *name;
auto *value = fdt_getprop_by_offset(fdt, prop, &name, &size);
```
and if `value == nullptr`, `size` is also be overwritten correctly but
`name` is not, which is quite inconsistent.
This commit makes sure `name` and `size` behavior consistently (reset to
reasonable value) when error occurs.
Signed-off-by: LoveSy <shana@zju.edu.cn>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
David Gibson