linux-kernel - Re: [PATCH 0/3] memblock: cleanup memblock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 23 Sep 2021 09:01:46 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Mike Rapoport <rppt@...nel.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        devicetree <devicetree@...r.kernel.org>,
        iommu <iommu@...ts.linux-foundation.org>,
        kasan-dev <kasan-dev@...glegroups.com>,
        KVM list <kvm@...r.kernel.org>,
        alpha <linux-alpha@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-efi <linux-efi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:BROADCOM NVRAM DRIVER" <linux-mips@...r.kernel.org>,
        Linux-MM <linux-mm@...ck.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        linux-s390 <linux-s390@...r.kernel.org>,
        Linux-sh list <linux-sh@...r.kernel.org>,
        "open list:SYNOPSYS ARC ARCHITECTURE" 
        <linux-snps-arc@...ts.infradead.org>,
        linux-um <linux-um@...ts.infradead.org>,
        linux-usb@...r.kernel.org,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        linux-sparc <sparclinux@...r.kernel.org>,
        xen-devel@...ts.xenproject.org, Mike Rapoport <rppt@...ux.ibm.com>
Subject: Re: [PATCH 0/3] memblock: cleanup memblock_free interface

On Thu, Sep 23, 2021 at 12:43 AM Mike Rapoport <rppt@...nel.org> wrote:
>
> The core change is in the third patch that makes memblock_free() a
> counterpart of memblock_alloc() and adds memblock_phys_alloc() to be a

^^^^^^^^^^^^^^^^^^^
> counterpart of memblock_phys_alloc().

That should be 'memblock_phys_free()'

HOWEVER.

The real reason I'm replying is that this patch is horribly buggy, and
will cause subtle problems that are nasty to debug.

You need to be a LOT more careful.

>From a trivial check - exactly because I looked at doing it with a
script, and decided it's not so easy - I found cases like this:

-               memblock_free(__pa(paca_ptrs) + new_ptrs_size,
+               memblock_free(paca_ptrs + new_ptrs_size,

which is COMPLETELY wrong.

Why? Because now that addition is done as _pointer_ addition, not as
an integer addition, and the end result is something completely
different.

pcac_ptrs is of type 'struct paca_struct **', so when you add
new_ptrs_size to it, it will add it in terms of that many pointers,
not that many bytes.

You need to use some smarter scripting, or some way to validate it.

And no, making the scripting just replace '__pa(x)' with '(void *)(x)'
- which _would_ be mindless and get the same result - is not
acceptable either, because it avoids one of the big improvements from
using the right interface, namely having compiler type checking (and
saner code that people understand).

So NAK. No broken automated scripting patches.

               Linus