linux-kernel - Re: [PATCH] vboxsf: fix old signature detection

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 27 Sep 2021 11:33:00 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Dan Carpenter <dan.carpenter@...cle.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Luc Van Oostenryck <luc.vanoostenryck@...il.com>,
        Sparse Mailing-list <linux-sparse@...r.kernel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        llvm@...ts.linux.dev
Subject: Re: [PATCH] vboxsf: fix old signature detection

On Mon, Sep 27, 2021 at 6:22 AM Arnd Bergmann <arnd@...nel.org> wrote:
>
> More specifically, ' think '\377' may be either -1 or 255 depending on
> the architecture.
> On most architectures, 'char' is implicitly signed, but on some others
> it is not.

Yeah. That code is just broken.

And Arnd, your patch may be "conceptually minimal", in that it keeps
thed broken code and makes it work. But it just dials up the oddity to
11.

The proper patch is just this appended thing that stops playing silly
games, and just uses "memcmp()".

I've verified that with sane build configurations, it just generates

        testq   %rsi, %rsi
        je      .L25
        cmpl    $-33620224, (%rsi)
        je      .L31

for that

        if (data && !memcmp(data, VBSF_MOUNT_SIGNATURE, 4)) {

test. With a lot of crazy debug options you'll actually see the
"memcmp()", but the bad code generation is the least of your options
in that case.

               Linus

View attachment "patch.diff" of type "text/x-patch" (1239 bytes)