Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAHk-=wg89U6PLp1AGhaqUx4KAZtkvKS6kuNmb+zObQhf1jez+g@mail.gmail.com>
 <20210329020746.GA250550@roeck-us.net>
In-Reply-To: <20210329020746.GA250550@roeck-us.net>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon, 29 Mar 2021 11:05:13 -0700
Message-ID: <CAHk-=wiMoP9PifpuUnQ3xmAM_LmGARr+fxFuOSX1rvh2mz35Mw@mail.gmail.com>
Subject: Re: Linux 5.12-rc5
To:     Guenter Roeck <linux@roeck-us.net>, Rob Herring <robh@kernel.org>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Mar 28, 2021 at 7:07 PM Guenter Roeck <linux@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-linux@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