linux-kernel - Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL

Open Source and information security mailing list archives

Message-ID: <20170803132035.gixd7m7oxtehpquu@yury-thinkpad>
Date:   Thu, 3 Aug 2017 16:20:35 +0300
From:   Yury Norov <ynorov@...iumnetworks.com>
To:     Matthias Kaehlcke <mka@...omium.org>
Cc:     zijun_hu <zijun_hu@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Mark Rutland <mark.rutland@....com>,
        Laura Abbott <labbott@...hat.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        linux-kernel@...r.kernel.org, Greg Hackmann <ghackmann@...gle.com>,
        Doug Anderson <dianders@...omium.org>,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL

Hi Matthias,

On Wed, Aug 02, 2017 at 03:51:59PM -0700, Matthias Kaehlcke wrote:
> As is the definition causes an integer overflow, which is expected,
> however clang raises the following warning:
> 
> arch/arm64/kernel/head.S:47:8: warning:
>       integer overflow in preprocessor expression
>   #elif (PAGE_OFFSET & 0x1fffff) != 0
>          ^~~~~~~~~~~
> arch/arm64/include/asm/memory.h:52:46: note:
>       expanded from macro 'PAGE_OFFSET'
>   #define PAGE_OFFSET             (UL(0xffffffffffffffff) << (VA_BITS - 1))
>                                       ~~~~~~~~~~~~~~~~~~  ^  ~~~~~~~~~~~~~
> 
> Use GENMASK_ULL() instead of shifting explicitly, the macro takes care
> of avoiding the overflow.
> 
> Reported-by: Nick Desaulniers <ndesaulniers@...gle.com>
> Signed-off-by: Matthias Kaehlcke <mka@...omium.org>
> ---
>  arch/arm64/include/asm/memory.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 32f82723338a..732d4eed8edd 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -65,7 +65,7 @@
>   */
>  #define VA_BITS			(CONFIG_ARM64_VA_BITS)
>  #define VA_START		(UL(0xffffffffffffffff) << VA_BITS)

IIUC VA_START should also produce warnings and should be also reworked.

> -#define PAGE_OFFSET		(UL(0xffffffffffffffff) << (VA_BITS - 1))
> +#define PAGE_OFFSET		GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)

The original type of PAGE_OFFSET is UL, and after your patch becomes ULL.
This is the same for arm64. But it would be less questionable if you
will specify it explicitly, or use GENMASK() instead of GENMASK_ULL().

>  #define KIMAGE_VADDR		(MODULES_END)
>  #define MODULES_END		(MODULES_VADDR + MODULES_VSIZE)
>  #define MODULES_VADDR		(VA_START + KASAN_SHADOW_SIZE)
> -- 
> 2.14.0.rc1.383.gd1ce394fe2-goog
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-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