On Tue, Nov 12, 2019 at 06:54:18PM -0700, Simon Glass wrote: > Add a new ASSUME_MASK option, which allows for some control over the > checks used in libfdt. With all assumptions enabled, libfdt assumes that > the input data and parameters are all correct and that internal errors > cannot happen. > > By default no assumptions are made and all checks are enabled. > > Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx> > --- > > Changes in v4: > - Add a can_assume() macros and squash the inline functions into one > - Drop unnecessary FDT_ prefix > - Fix 'Incosistencies' typo > - Merge the 'friendly' and 'sane' checks into one > - Update and expand comments > > Changes in v3: > - Expand the comments about each check option > - Invert the checks to be called assumptions > - Move header-version code to fdt.c > - Move the comments on check options to the enum > - Use hex for CHK_MASK > > Changes in v2: > - Add an fdt_ prefix to avoid namespace conflicts > - Use symbolic names for _check functions and drop leading underscores > > Makefile | 6 ++- > libfdt/libfdt_internal.h | 88 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 93 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 4d88157..bdeaf93 100644 > --- a/Makefile > +++ b/Makefile > @@ -16,7 +16,11 @@ EXTRAVERSION = > LOCAL_VERSION = > CONFIG_LOCALVERSION = > > -CPPFLAGS = -I libfdt -I . > +# Control the assumptions made (e.g. risking security issues) in the code. > +# See libfdt_internal.h for details > +ASSUME_MASK ?= 0 > + > +CPPFLAGS = -I libfdt -I . -DFDT_ASSUME_MASK=$(ASSUME_MASK) > WARNINGS = -Wall -Wpointer-arith -Wcast-qual -Wnested-externs \ > -Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls -Wshadow > CFLAGS = -g -Os $(SHAREDLIB_CFLAGS) -Werror $(WARNINGS) $(EXTRA_CFLAGS) > diff --git a/libfdt/libfdt_internal.h b/libfdt/libfdt_internal.h > index 058c735..198480c 100644 > --- a/libfdt/libfdt_internal.h > +++ b/libfdt/libfdt_internal.h > @@ -48,4 +48,92 @@ static inline struct fdt_reserve_entry *fdt_mem_rsv_w_(void *fdt, int n) > > #define FDT_SW_MAGIC (~FDT_MAGIC) > > +/**********************************************************************/ > +/* Checking controls */ > +/**********************************************************************/ > + > +#ifndef FDT_ASSUME_MASK > +#define FDT_ASSUME_MASK 0 > +#endif > + > +/* > + * Defines assumptions which can be enabled. Each of these can be enabled > + * individually. For maximum saftey, don't enable any assumptions! > + * > + * For minimal code size and no safety, use ASSUME_PERFECT at your own risk. > + * You should have another method of validating the device tree, such as a > + * signature or hash check before using libfdt. > + * > + * For situations where security is not a concern it may be safe to enable > + * ASSUME_SANE. > + */ > +enum { > + /* > + * This does essentially no checks. Only the latest device-tree > + * version is correctly handled. Inconsistencies or errors in the device > + * tree may cause undefined behaviour or crashes. Likewise for errors in parameters, yes? > + * If an error occurs when modifying the tree it may leave the tree in > + * an intermediate (but valid) state. As an example, adding a property > + * where there is insufficient space may result in the property name > + * being added to the string table even though the property itself is > + * not added to the struct section. > + * > + * Only use this if you have a fully validated device tree with > + * the latest supported version and wish to minimise code size. > + */ > + ASSUME_PERFECT = 0xff, > + > + /* > + * This assumes that the device tree is sane. i.e. header metadata > + * and basic hierarchy are correct. > + * > + * It also assumes libfdt functions are called with valid parameters, > + * i.e. not trigger FDT_ERR_BADOFFSET or offsets that are out of bounds. > + * It disables any extensive checking of parameters and the device > + * tree, making various assumptions about correctness. Normal device > + * trees produced by libfdt and the compiler should be handled safely. > + * Malicious device trees and complete garbage may cause libfdt to > + * behave badly or crash. > + * > + * These checks will be sufficient if you have a valid device tree with > + * no internal inconsistencies and call libfdt with correct parameters. > + * With this assumption, libfdt will generally not return > + * -FDT_ERR_INTERNAL, -FDT_ERR_BADLAYOUT, etc. > + */ > + ASSUME_SANE = 1 << 0, How tricky would it be to split this into ASSUME_VALID_DTB and ASSUME_VALID_PARAMETERS? I don't know useful those are on their own, but it seems to me it might be easier to define the effect of this if split that way. > + > + /* > + * This disables checks for device-tree version and removes all code > + * which handles older versions. > + * > + * Only enable this if you know you have a device tree with the latest > + * version. > + */ > + ASSUME_LATEST = 1 << 1, > + > + /* > + * This assume that it is OK for a failed additional to the device tree > + * due to lack of space or some other problem can skip any rollback > + * steps (such as dropping the property name from the string table). > + * This is safe to enable in most circumstances, even though it may > + * leave the tree in a sub-optimal state. > + */ > + ASSUME_NO_ROLLBACK = 1 << 2, > +}; > + > +/** > + * _can_assume() - check if a particular assumption is enabled Don't use a leading _ please, that's reserved for system libraries. I've moved to trailing _ for that reason. > + * > + * @mask: Mask to check (ASSUME_...) > + * @return true if that assumption is enabled, else false > + */ > +static inline bool _can_assume(int mask) > +{ > + return FDT_ASSUME_MASK & mask; > +} > + > +/** helper macros for checking assumptions */ > +#define can_assume(_assume) _can_assume(ASSUME_ ## _assume) > + > #endif /* LIBFDT_INTERNAL_H */ -- 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