On Sun, Mar 28, 2021 at 7:07 PM Guenter Roeck <li...@roeck-us.net> wrote: > > This is not really a new problem. I enabled devicetree unit tests > in the openrisc kernel and was rewarded with a crash. > https://lore.kernel.org/lkml/20210327224116.69309-1-li...@roeck-us.net/ > has all the glorious details.
Hmm. I'm not sure I love that patch. I don't think the patch is _wrong_ per se, but if that "require 8 byte alignment" is a problem, then this seems to be papering over the issue rather than fixing it. So your patch protects from a NULL pointer dereference, but the underlying issue seems to be a regression, and the fix sounds like the kernel shouldn't be so strict about alignment requirements. I guess we could make ARCH_SLAB_MINALIGN be at least 8 (perhaps only if the allocations is >= 8) but honestly, I don't think libfdt merits making such a big change. Small allocations are actually not uncommon in the kernel, and on 32-bit architectures I think 4-byte allocations are normal. So I'd be inclined to just remove the new /* The device tree must be at an 8-byte aligned address */ if ((uintptr_t)fdt & 7) return -FDT_ERR_ALIGNMENT; check in scripts/dtc/libfdt/fdt.c which I assume is the source of the problem. Rob? Your patch to then avoid the NULL pointer dereference seems to be then an additional safety, but not fixing the actual regression. Linus