Browse Source

libfdt: Tweak data handling to satisfy Coverity

In libfdt we often sanity test fdt_totalsize(fdt) fairly early, then
trust it (but *only* that header field) for the remainder of our work.
However, Coverity gets confused by this - it sees the byteswap in
fdt32_ld() and assumes that means it is coming from an untrusted source
everytime, resulting in many tainted data warnings.

Most of these end up with logic in fdt_get_string() as the unsafe
destination for this tainted data, so let's tweak the logic there to make
it clearer to Coverity that this is ok.

We add a sanity test on fdt_totalsize() to fdt_probe_ro_().  Because the
interface allows bare ints to be used for offsets, we already have the
assumption that totalsize must be 31-bits or less (2GiB would be a
ludicrously large fdt).  This makes this more explicit.

We also make fdt_probe_ro() return the size for convenience, and change the
logic in fdt_get_string() to keep it in a local so that Coverity can see
that it has already been bounds-checked.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
main
David Gibson 6 years ago
parent
commit
812b1956a0
  1. 9
      libfdt/fdt.c
  2. 11
      libfdt/fdt_ro.c
  3. 10
      libfdt/libfdt_internal.h

9
libfdt/fdt.c

@ -15,8 +15,10 @@
* that the given buffer contains what appears to be a flattened * that the given buffer contains what appears to be a flattened
* device tree with sane information in its header. * device tree with sane information in its header.
*/ */
int fdt_ro_probe_(const void *fdt) int32_t fdt_ro_probe_(const void *fdt)
{ {
uint32_t totalsize = fdt_totalsize(fdt);

if (fdt_magic(fdt) == FDT_MAGIC) { if (fdt_magic(fdt) == FDT_MAGIC) {
/* Complete tree */ /* Complete tree */
if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION) if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
@ -31,7 +33,10 @@ int fdt_ro_probe_(const void *fdt)
return -FDT_ERR_BADMAGIC; return -FDT_ERR_BADMAGIC;
} }


return 0; if (totalsize < INT32_MAX)
return totalsize;
else
return -FDT_ERR_TRUNCATED;
} }


static int check_off_(uint32_t hdrsize, uint32_t totalsize, uint32_t off) static int check_off_(uint32_t hdrsize, uint32_t totalsize, uint32_t off)

11
libfdt/fdt_ro.c

@ -33,19 +33,20 @@ static int fdt_nodename_eq_(const void *fdt, int offset,


const char *fdt_get_string(const void *fdt, int stroffset, int *lenp) const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
{ {
int32_t totalsize = fdt_ro_probe_(fdt);
uint32_t absoffset = stroffset + fdt_off_dt_strings(fdt); uint32_t absoffset = stroffset + fdt_off_dt_strings(fdt);
size_t len; size_t len;
int err; int err;
const char *s, *n; const char *s, *n;


err = fdt_ro_probe_(fdt); err = totalsize;
if (err != 0) if (totalsize < 0)
goto fail; goto fail;


err = -FDT_ERR_BADOFFSET; err = -FDT_ERR_BADOFFSET;
if (absoffset >= fdt_totalsize(fdt)) if (absoffset >= totalsize)
goto fail; goto fail;
len = fdt_totalsize(fdt) - absoffset; len = totalsize - absoffset;


if (fdt_magic(fdt) == FDT_MAGIC) { if (fdt_magic(fdt) == FDT_MAGIC) {
if (stroffset < 0) if (stroffset < 0)
@ -288,7 +289,7 @@ const char *fdt_get_name(const void *fdt, int nodeoffset, int *len)
const char *nameptr; const char *nameptr;
int err; int err;


if (((err = fdt_ro_probe_(fdt)) != 0) if (((err = fdt_ro_probe_(fdt)) < 0)
|| ((err = fdt_check_node_offset_(fdt, nodeoffset)) < 0)) || ((err = fdt_check_node_offset_(fdt, nodeoffset)) < 0))
goto fail; goto fail;



10
libfdt/libfdt_internal.h

@ -11,11 +11,11 @@
#define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE)) #define FDT_TAGALIGN(x) (FDT_ALIGN((x), FDT_TAGSIZE))


int fdt_ro_probe_(const void *fdt); int fdt_ro_probe_(const void *fdt);
#define FDT_RO_PROBE(fdt) \ #define FDT_RO_PROBE(fdt) \
{ \ { \
int err_; \ int totalsize_; \
if ((err_ = fdt_ro_probe_(fdt)) != 0) \ if ((totalsize_ = fdt_ro_probe_(fdt)) < 0) \
return err_; \ return totalsize_; \
} }


int fdt_check_node_offset_(const void *fdt, int offset); int fdt_check_node_offset_(const void *fdt, int offset);

Loading…
Cancel
Save