Hi Simon, On Fri, Oct 06, 2023 at 07:22:14AM -0600, Simon Glass wrote: > Hi Pierre-Clément, > > On Fri, 6 Oct 2023 at 06:48, Pierre-Clément Tosi <ptosi@xxxxxxxxxx> wrote: > > > > + if (namelen < 1) > > + return -FDT_ERR_BADPATH; > > + > > This would be end == path, right? Would it be better to check that? > Are you worried about negative numbers? > The main focus of this patch was namelen==0 (e.g. fdt_path_offset(fdt, "")) which violates the spec and makes the function return the offset of the root node, a counter-intuitive result which, if used internally (other libfdt functions call fdt_path_offset()), could lead to unintended behavior. Furthermore, under the right conditions, fdt_path_offset(fdt, "") may lead to a stack overflow attack, which this patch addresses but which is also separately addressed by [1] (although one doesn't make the other redundant). As I was adding a check on namelen, I took the opportunity to also reject all negative values. Do you recommend I only reject end == path and accept/ignore negative lengths? If this validation makes sense, v2 will add coverage for it in 'make check'. [1]: https://lore.kernel.org/devicetree-compiler/20231007110710.i2oj24oirdtyt5m4@xxxxxxxxxx > > /* see if we have an alias */ > > if (*path != '/') { > > const char *q = memchr(path, '/', end - p); > > -- > > Regards, > Simon Thanks, -- Pierre
