2017-08-02 22:52:37

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH 1/2] bitops: Avoid integer overflow warning in GENMASK_ULL

GENMASK_ULL performs a left-shift of (~0ULL), which technically
results in an integer overflow. clang raises a warning about
this if the overflow occurs in a preprocessor expression. To
avoid the overflow first perform a right-shift to clear the
bits that are shifted out.

Signed-off-by: Matthias Kaehlcke <[email protected]>
---
include/linux/bitops.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index a83c822c35c2..21dfe63001e3 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -22,7 +22,7 @@
(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))

#define GENMASK_ULL(h, l) \
- (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
+ ((((~0ULL) >> (l)) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))

extern unsigned int __sw_hweight8(unsigned int w);
extern unsigned int __sw_hweight16(unsigned int w);
--
2.14.0.rc1.383.gd1ce394fe2-goog


2017-08-02 22:52:50

by Matthias Kaehlcke

[permalink] [raw]
Subject: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL

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 <[email protected]>
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
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)
-#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
+#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
#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

2017-08-02 23:13:15

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL

don't forget to include linux/bitops.h now in memory.h

/usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/kernel/head.S:47:8:
error:
function-like macro 'GENMASK_ULL' is not defined
#elif (PAGE_OFFSET & 0x1fffff) != 0
^
/usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/include/asm/memory.h:52:22:
note:

expanded from macro 'PAGE_OFFSET'
#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
^
1 error generated.

On Wed, Aug 2, 2017 at 3:51 PM, Matthias Kaehlcke <[email protected]> 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 <[email protected]>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> 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)
> -#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
> +#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
> #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
>



--
Thanks,
~Nick Desaulniers

2017-08-02 23:19:14

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL

hmm, seems including the definition of GENMASK_ULL causes tons of issues
see definition of UL() macro
defines _AC
token pastes UL on literal when not assembly
so looks like GENMASK_ULL is not ready to be used from assembly

On Wed, Aug 2, 2017 at 4:13 PM, Nick Desaulniers
<[email protected]> wrote:
> don't forget to include linux/bitops.h now in memory.h
>
> /usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/kernel/head.S:47:8:
> error:
> function-like macro 'GENMASK_ULL' is not defined
> #elif (PAGE_OFFSET & 0x1fffff) != 0
> ^
> /usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/include/asm/memory.h:52:22:
> note:
>
> expanded from macro 'PAGE_OFFSET'
> #define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
> ^
> 1 error generated.
>
> On Wed, Aug 2, 2017 at 3:51 PM, Matthias Kaehlcke <[email protected]> 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 <[email protected]>
>> Signed-off-by: Matthias Kaehlcke <[email protected]>
>> ---
>> 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)
>> -#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
>> +#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
>> #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
>>
>
>
>
> --
> Thanks,
> ~Nick Desaulniers



--
Thanks,
~Nick Desaulniers

2017-08-02 23:44:35

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL

El Wed, Aug 02, 2017 at 04:19:11PM -0700 Nick Desaulniers ha dit:

> hmm, seems including the definition of GENMASK_ULL causes tons of issues
> see definition of UL() macro
> defines _AC
> token pastes UL on literal when not assembly
> so looks like GENMASK_ULL is not ready to be used from assembly

sorry, it seems I only did a partial build when testing this :/

> On Wed, Aug 2, 2017 at 4:13 PM, Nick Desaulniers
> <[email protected]> wrote:
> > don't forget to include linux/bitops.h now in memory.h
> >
> > /usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/kernel/head.S:47:8:
> > error:
> > function-like macro 'GENMASK_ULL' is not defined
> > #elif (PAGE_OFFSET & 0x1fffff) != 0
> > ^
> > /usr/local/google/home/ndesaulniers/android/kernel-wahoo/private/msm-google/arch/arm64/include/asm/memory.h:52:22:
> > note:
> >
> > expanded from macro 'PAGE_OFFSET'
> > #define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
> > ^
> > 1 error generated.
> >
> > On Wed, Aug 2, 2017 at 3:51 PM, Matthias Kaehlcke <[email protected]> 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 <[email protected]>
> >> Signed-off-by: Matthias Kaehlcke <[email protected]>
> >> ---
> >> 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)
> >> -#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
> >> +#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
> >> #define KIMAGE_VADDR (MODULES_END)
> >> #define MODULES_END (MODULES_VADDR + MODULES_VSIZE)
> >> #define MODULES_VADDR (VA_START + KASAN_SHADOW_SIZE)
> >>
> >
> >
> >
>
>
>

2017-08-03 13:21:02

by Yury Norov

[permalink] [raw]
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 <[email protected]>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2017-08-03 13:25:27

by Yury Norov

[permalink] [raw]
Subject: Re: [PATCH 1/2] bitops: Avoid integer overflow warning in GENMASK_ULL

On Wed, Aug 02, 2017 at 03:51:58PM -0700, Matthias Kaehlcke wrote:
> GENMASK_ULL performs a left-shift of (~0ULL), which technically
> results in an integer overflow. clang raises a warning about
> this if the overflow occurs in a preprocessor expression. To
> avoid the overflow first perform a right-shift to clear the
> bits that are shifted out.
>
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> include/linux/bitops.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index a83c822c35c2..21dfe63001e3 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -22,7 +22,7 @@
> (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))

Once you touche GENMASK_ULL, why not to fix also GENMASK.

>
> #define GENMASK_ULL(h, l) \
> - (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
> + ((((~0ULL) >> (l)) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
>
> extern unsigned int __sw_hweight8(unsigned int w);
> extern unsigned int __sw_hweight16(unsigned int w);
> --
> 2.14.0.rc1.383.gd1ce394fe2-goog
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2017-08-03 17:03:59

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH 1/2] bitops: Avoid integer overflow warning in GENMASK_ULL

El Thu, Aug 03, 2017 at 04:24:56PM +0300 Yury Norov ha dit:

> On Wed, Aug 02, 2017 at 03:51:58PM -0700, Matthias Kaehlcke wrote:
> > GENMASK_ULL performs a left-shift of (~0ULL), which technically
> > results in an integer overflow. clang raises a warning about
> > this if the overflow occurs in a preprocessor expression. To
> > avoid the overflow first perform a right-shift to clear the
> > bits that are shifted out.
> >
> > Signed-off-by: Matthias Kaehlcke <[email protected]>
> > ---
> > include/linux/bitops.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> > index a83c822c35c2..21dfe63001e3 100644
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -22,7 +22,7 @@
> > (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
>
> Once you touche GENMASK_ULL, why not to fix also GENMASK.

Will do, thanks.

Even though GENMASK_ULL can't be used (as is) to define the arm64
PAGE_OFFSET as initially intended it seems the change is still
worthwhile since there are a few other preprocessor expressions using
GENMASK_ULL.

2017-08-03 17:11:54

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH] arm64: avoid overflow in VA_START and PAGE_OFFSET

The bitmask used to define these values produces overflow, as seen by
this compiler 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))
~~~~~~~~~~~~~~~~~~ ^

It would be preferrable to use GENMASK_ULL() instead, but it's not set
up to be used from assembly (the UL() macro token pastes UL suffixes
when not included in assembly sources).

Suggested-by: Yury Norov <[email protected]>
Suggested-by: Matthias Kaehlcke <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
arch/arm64/include/asm/memory.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 32f82723338a..dde717a31dee 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -64,8 +64,9 @@
* TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area.
*/
#define VA_BITS (CONFIG_ARM64_VA_BITS)
-#define VA_START (UL(0xffffffffffffffff) << VA_BITS)
-#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
+#define VA_START ((UL(0xffffffffffffffff) >> VA_BITS) << VA_BITS)
+#define PAGE_OFFSET ((UL(0xffffffffffffffff) >> (VA_BITS - 1)) \
+ << (VA_BITS - 1))
#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

2017-08-03 17:20:35

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] arm64: avoid overflow in VA_START and PAGE_OFFSET

On 3 August 2017 at 18:11, Nick Desaulniers <[email protected]> wrote:
> The bitmask used to define these values produces overflow, as seen by
> this compiler 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))
> ~~~~~~~~~~~~~~~~~~ ^
>
> It would be preferrable to use GENMASK_ULL() instead, but it's not set
> up to be used from assembly (the UL() macro token pastes UL suffixes
> when not included in assembly sources).
>
> Suggested-by: Yury Norov <[email protected]>
> Suggested-by: Matthias Kaehlcke <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
> ---
> arch/arm64/include/asm/memory.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index 32f82723338a..dde717a31dee 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -64,8 +64,9 @@
> * TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area.
> */
> #define VA_BITS (CONFIG_ARM64_VA_BITS)
> -#define VA_START (UL(0xffffffffffffffff) << VA_BITS)
> -#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
> +#define VA_START ((UL(0xffffffffffffffff) >> VA_BITS) << VA_BITS)
> +#define PAGE_OFFSET ((UL(0xffffffffffffffff) >> (VA_BITS - 1)) \
> + << (VA_BITS - 1))
> #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
>

Would

#define VA_START (UL(0xffffffffffffffff) - (1 << VA_BITS) + 1)

also work?

2017-08-03 17:53:14

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] arm64: avoid overflow in VA_START and PAGE_OFFSET

> Would
>
> #define VA_START (UL(0xffffffffffffffff) - (1 << VA_BITS) + 1)
>
> also work?

I think you'd have to wrap the 1 in a UL(), ex:

#define VA_START (UL(0xffffffffffffffff) - (UL(1) << VA_BITS) + 1)

Otherwise IIUC a integral literal (`1`) is treated as an int, which on
arm64 is LP64 making it 32b, where most configs set VA_BITS
to larger than 32b. Shifting by more than the width is undefined
behavior. And without it, I get compile errors.

I'll send v2 with your suggestion, thanks.
--
Thanks,
~Nick Desaulniers

2017-08-03 18:04:30

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH v2] arm64: avoid overflow in VA_START and PAGE_OFFSET

The bitmask used to define these values produces overflow, as seen by
this compiler 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))
~~~~~~~~~~~~~~~~~~ ^

It would be preferrable to use GENMASK_ULL() instead, but it's not set
up to be used from assembly (the UL() macro token pastes UL suffixes
when not included in assembly sources).

Suggested-by: Ard Biesheuvel <[email protected]>
Suggested-by: Yury Norov <[email protected]>
Suggested-by: Matthias Kaehlcke <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
arch/arm64/include/asm/memory.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 32f82723338a..ef39dcb9ca6a 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -64,8 +64,10 @@
* TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area.
*/
#define VA_BITS (CONFIG_ARM64_VA_BITS)
-#define VA_START (UL(0xffffffffffffffff) << VA_BITS)
-#define PAGE_OFFSET (UL(0xffffffffffffffff) << (VA_BITS - 1))
+#define VA_START (UL(0xffffffffffffffff) - \
+ (UL(1) << VA_BITS) + 1)
+#define PAGE_OFFSET (UL(0xffffffffffffffff) - \
+ (UL(1) << (VA_BITS - 1)) + 1)
#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

2017-08-04 02:27:06

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: Define PAGE_OFFSET using GENMASK_ULL

Hi Matthias,

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on v4.13-rc3 next-20170803]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/bitops-Avoid-integer-overflow-warning-in-GENMASK_ULL/20170803-230211
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64

All error/warnings (new ones prefixed by >>):

In file included from arch/arm64/include/asm/pgtable.h:22:0,
from arch/arm64/include/asm/kernel-pgtable.h:22,
from arch/arm64/kernel/head.S:34:
>> arch/arm64/include/asm/memory.h:68:33: error: missing binary operator before token "("
#define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
^
>> arch/arm64/kernel/head.S:51:8: note: in expansion of macro 'PAGE_OFFSET'
#elif (PAGE_OFFSET & 0x1fffff) != 0
^~~~~~~~~~~
--
In file included from include/linux/printk.h:6:0,
from include/linux/kernel.h:13,
from arch/arm64/mm/init.c:20:
arch/arm64/mm/init.c: In function 'mem_init':
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
>> arch/arm64/mm/init.c:614:2: note: in expansion of macro 'pr_notice'
pr_notice(" vmalloc : 0x%16lx - 0x%16lx (%6ld GB)\n",
^~~~~~~~~
>> include/linux/kern_levels.h:4:18: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
>> arch/arm64/mm/init.c:614:2: note: in expansion of macro 'pr_notice'
pr_notice(" vmalloc : 0x%16lx - 0x%16lx (%6ld GB)\n",
^~~~~~~~~
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 2 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:626:2: note: in expansion of macro 'pr_notice'
pr_notice(" fixed : 0x%16lx - 0x%16lx (%6ld KB)\n",
^~~~~~~~~
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:626:2: note: in expansion of macro 'pr_notice'
pr_notice(" fixed : 0x%16lx - 0x%16lx (%6ld KB)\n",
^~~~~~~~~
>> include/linux/kern_levels.h:4:18: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:626:2: note: in expansion of macro 'pr_notice'
pr_notice(" fixed : 0x%16lx - 0x%16lx (%6ld KB)\n",
^~~~~~~~~
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 2 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:628:2: note: in expansion of macro 'pr_notice'
pr_notice(" PCI I/O : 0x%16lx - 0x%16lx (%6ld MB)\n",
^~~~~~~~~
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:628:2: note: in expansion of macro 'pr_notice'
pr_notice(" PCI I/O : 0x%16lx - 0x%16lx (%6ld MB)\n",
^~~~~~~~~
>> include/linux/kern_levels.h:4:18: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:628:2: note: in expansion of macro 'pr_notice'
pr_notice(" PCI I/O : 0x%16lx - 0x%16lx (%6ld MB)\n",
^~~~~~~~~
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 2 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:631:2: note: in expansion of macro 'pr_notice'
pr_notice(" vmemmap : 0x%16lx - 0x%16lx (%6ld GB maximum)\n",
^~~~~~~~~
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:631:2: note: in expansion of macro 'pr_notice'
pr_notice(" vmemmap : 0x%16lx - 0x%16lx (%6ld GB maximum)\n",
^~~~~~~~~
>> include/linux/kern_levels.h:4:18: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:631:2: note: in expansion of macro 'pr_notice'
pr_notice(" vmemmap : 0x%16lx - 0x%16lx (%6ld GB maximum)\n",
^~~~~~~~~
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 2 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:637:2: note: in expansion of macro 'pr_notice'
pr_notice(" memory : 0x%16lx - 0x%16lx (%6ld MB)\n",
^~~~~~~~~
>> include/linux/kern_levels.h:4:18: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:12:21: note: in expansion of macro 'KERN_SOH'
#define KERN_NOTICE KERN_SOH "5" /* normal but significant condition */
^~~~~~~~
>> include/linux/printk.h:306:9: note: in expansion of macro 'KERN_NOTICE'
printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
arch/arm64/mm/init.c:637:2: note: in expansion of macro 'pr_notice'
pr_notice(" memory : 0x%16lx - 0x%16lx (%6ld MB)\n",
^~~~~~~~~
--
In file included from include/linux/printk.h:6:0,
from include/linux/kernel.h:13,
from include/asm-generic/bug.h:15,
from arch/arm64/include/asm/bug.h:66,
from include/linux/bug.h:4,
from include/linux/mmdebug.h:4,
from include/linux/mm.h:8,
from include/linux/mman.h:4,
from arch/arm64/kvm/../../../virt/kvm/arm/mmu.c:19:
arch/arm64/kvm/../../../virt/kvm/arm/mmu.c: In function 'kvm_mmu_init':
include/linux/kern_levels.h:4:18: warning: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'long long unsigned int' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
include/linux/kern_levels.h:13:19: note: in expansion of macro 'KERN_SOH'
#define KERN_INFO KERN_SOH "6" /* informational */
^~~~~~~~
include/linux/printk.h:308:9: note: in expansion of macro 'KERN_INFO'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~
>> include/linux/kvm_host.h:446:2: note: in expansion of macro 'pr_info'
pr_info("kvm [%i]: " fmt, task_pid_nr(current), ## __VA_ARGS__)
^~~~~~~
>> arch/arm64/kvm/../../../virt/kvm/arm/mmu.c:1754:2: note: in expansion of macro 'kvm_info'
kvm_info("HYP VA range: %lx:%lx\n",
^~~~~~~~

vim +68 arch/arm64/include/asm/memory.h

56
57 /*
58 * PAGE_OFFSET - the virtual address of the start of the linear map (top
59 * (VA_BITS - 1))
60 * KIMAGE_VADDR - the virtual address of the start of the kernel image
61 * VA_BITS - the maximum number of bits for virtual addresses.
62 * VA_START - the first kernel virtual address.
63 * TASK_SIZE - the maximum size of a user space task.
64 * TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area.
65 */
66 #define VA_BITS (CONFIG_ARM64_VA_BITS)
67 #define VA_START (UL(0xffffffffffffffff) << VA_BITS)
> 68 #define PAGE_OFFSET GENMASK_ULL(BITS_PER_LONG_LONG - 1, VA_BITS - 1)
69 #define KIMAGE_VADDR (MODULES_END)
70 #define MODULES_END (MODULES_VADDR + MODULES_VSIZE)
71 #define MODULES_VADDR (VA_START + KASAN_SHADOW_SIZE)
72 #define MODULES_VSIZE (SZ_128M)
73 #define VMEMMAP_START (PAGE_OFFSET - VMEMMAP_SIZE)
74 #define PCI_IO_END (VMEMMAP_START - SZ_2M)
75 #define PCI_IO_START (PCI_IO_END - PCI_IO_SIZE)
76 #define FIXADDR_TOP (PCI_IO_START - SZ_2M)
77 #define TASK_SIZE_64 (UL(1) << VA_BITS)
78

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (13.51 kB)
.config.gz (34.39 kB)
Download all attachments