On Tue, Nov 05, 2019 at 05:46:02AM -0700, Simon Glass wrote: > Hi David, > > On Mon, 4 Nov 2019 at 10:06, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > > > > On Thu, Oct 24, 2019 at 09:29:24PM -0600, Simon Glass wrote: > > > Allow enabling FDT_ASSUME_FRIENDLY to disable extra checks for invalid > > > device tree files. > > > > The distinction between what's covered here and what's covered by > > chk_basic() isn't very obvious to me. They both seem to be covering > > checks of fundamental invarants of the format. > > Perhaps this is just a matter of degree (and amount of code)? I don't see a lot of value in having a check level that will catch some, but not all of a particular class of errors (though certainly what constitutes a "class of errors" is pretty debatable). > Basic checking assumes that the data can be parsed and strings/lengths > are consistent. But the tree might have bad hierarchy, perhaps and it > would catch that. Things like -FDT_ERR_BADLAYOUT. > > Extra checking goes further, checking every length and string pointer, > but with a higher code cost. Hm, so. From a code size point of view, checking header sanity but not property sanity seems like a fair trade off. But, I don't think it really buys you anything. A bug causing bad offsets in one of the many string offsets seems more likely that one causing bad offsets in the handful of header fields. So from a safety point of view, I don't think checking one but not the other is particularly useful - and if you did, the one you're considering "extra" actually seems like a higher priority that the "basic" tests. So I think "trust all dtb internal offsets" should be under a single assumption bit. > Most of the latter code was added recently and there was already a > fair amount of checking code. So I feel this is 'more paranoid' than > the older code. Before this 'extra' code the code size of libfdt was > quite a bit smaller, so I feel there is some value in having two > separate levels for checking. > > What do you think? It happened that way historically because I always intended it to check thoroughly, but I then realized I hadn't considered a lot of cases. From that point of view the new checks were merely missing before, rather than being "extra". -- 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
