linux-kernel - Re: [PATCH v1 8/8] ACPI: Use recently introduced uuid_le_cmp

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:   Thu, 27 Apr 2017 14:46:31 +0200
From:   Borislav Petkov <bp@...e.de>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        alsa-devel@...a-project.org, linux-input@...r.kernel.org,
        kvm@...r.kernel.org, devel@...uxdriverproject.org,
        linux-efi@...r.kernel.org, linux-acpi@...r.kernel.org
Subject: Re: [PATCH v1 8/8] ACPI: Use recently introduced uuid_le_cmp_p{p}()
 helpers

On Fri, Apr 21, 2017 at 11:22:31PM +0200, Rafael J. Wysocki wrote:
> >  #ifdef CONFIG_ACPI_APEI_PCIEAER
> > -		else if (!uuid_le_cmp(*(uuid_le *)gdata->section_type,
> > -				      CPER_SEC_PCIE)) {
> > +		else if (!uuid_le_cmp_p(sec_type, CPER_SEC_PCIE)) {
> >  			struct cper_sec_pcie *pcie_err;
> >  			pcie_err = (struct cper_sec_pcie *)(gdata+1);
> >  			if (sev == GHES_SEV_RECOVERABLE &&
> > 
> 
> But this one is for Boris.

I don't see anything wrong with it upon a brief inspection.

What could be improved here, though, is if the whole uuid_* types
handling be changed so that gcc doesn't generate yucky code. Because
here's what it does now, regardless of this patch:

        .file 16 "./include/linux/uuid.h"
        .loc 16 63 0
        leaq    16(%rsp), %rsi  #,
        movl    $16, %edx       #,
        movq    %r15, %rdi      # gdata,
        movb    $84, 16(%rsp)   #, MEM[(struct  *)&u2]
        movb    $-23, 17(%rsp)  #, MEM[(struct  *)&u2 + 1B]
        movb    $-107, 18(%rsp) #, MEM[(struct  *)&u2 + 2B]
        movb    $-39, 19(%rsp)  #, MEM[(struct  *)&u2 + 3B]
        movb    $-63, 20(%rsp)  #, MEM[(struct  *)&u2 + 4B]
        movb    $-69, 21(%rsp)  #, MEM[(struct  *)&u2 + 5B]
        movb    $15, 22(%rsp)   #, MEM[(struct  *)&u2 + 6B]
        movb    $67, 23(%rsp)   #, MEM[(struct  *)&u2 + 7B]
        movb    $-83, 24(%rsp)  #, MEM[(struct  *)&u2 + 8B]
        movb    $-111, 25(%rsp) #, MEM[(struct  *)&u2 + 9B]
        movb    $-76, 26(%rsp)  #, MEM[(struct  *)&u2 + 10B]
        movb    $77, 27(%rsp)   #, MEM[(struct  *)&u2 + 11B]
        movb    $-53, 28(%rsp)  #, MEM[(struct  *)&u2 + 12B]
        movb    $60, 29(%rsp)   #, MEM[(struct  *)&u2 + 13B]
        movb    $111, 30(%rsp)  #, MEM[(struct  *)&u2 + 14B]
        movb    $53, 31(%rsp)   #, MEM[(struct  *)&u2 + 15B]
        call    memcmp  #

So it is basically building that UUID byte by byte before calling
memcmp.

And I'm wondering if those 16-byte arrays could be replaced with

typedef struct {
        u64 a, b;
} u128;

from the crypto code.

And whether the code generated by gcc would look much saner. Because the
CPU can handle two qwords much better/faster than 16 u8s.

Anyway, in case someone feels bored...

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--