linux-kernel - Re: [PATCH 6/7] RISC-V: arch/riscv/kernel

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]

Message-ID: <CAK8P3a1z2s6UZAUS4stM25LcM-LR8Ts-HtNha-5xxKk0gjUnaA@mail.gmail.com>
Date:   Thu, 25 May 2017 21:51:54 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Palmer Dabbelt <palmer@...belt.com>
Cc:     Olof Johansson <olof@...om.net>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        albert@...ive.com
Subject: Re: [PATCH 6/7] RISC-V: arch/riscv/kernel

On Thu, May 25, 2017 at 3:59 AM, Palmer Dabbelt <palmer@...belt.com> wrote:
> On Mon, 22 May 2017 19:11:35 PDT (-0700), olof@...om.net wrote:

>  * Parens around single-statement __asm__ macros.  For these I also get a
>    message when they're wrapped in "do {} while (0)", so I'm not sure what else
>    to do.

I would generally recommend using inline functions for those, and only do
macros when you need them.

>  * Parens around macros like "#define RISCV_PTR .dword".  These can't have
>    parens because they go directly to the assembler, so I'm considering this a
>    false-positive.

agreed

>  * Warnings about volatile in function declarations in bitops.h.  These are
>    copied from other architectures.  There were a handful of other volatiles
>    that I fixed,, but I think these should stay.

Agreed, bitops.h is one of the few headers that should use 'volatile'.

>  * Definitions like ARCH_HAS_SETUP_ADDITIONAL_PAGES, these are also present in
>    other architectures.

What is the warning here? I would assume that you should leave this
unchanged as well.

>  * We added new typedefs, I can remove these if that's a problem.  They're
>    there to match our other code (bootloader and simulator).

It depends. What typedefs are those? Removing the typedefs in both
the kernel and the other code that uses the same types is likely the
best option here.

>  * A handful of lines over 80 characters that I think are onerous to break any
>    more.

Right, don't worry about it too much, and use common sense for this
warning.

>  * Some warnings about printk() not having a KERN_ prefix.  I fixed a handful
>    of these, but the remaining ones I don't know how to fix (in show_regs, for
>    example, where arm64 also has them).

KERN_CONT

>  * Extern declarations in C files, all of which link to symbols in assembly or
>    linker scripts.  These were copied from other architectures.

I would try to fix those by using a header even if there is only one user.
I'd actually like to get a compile-time warning for those in the long run,
maybe with 'make W=1', so better don't introduce new ones.

     Arnd