Re: [PATCH 07/10] mm: correct sign-extension issue in MMF_* flag masks

[Liam R. Howlett]

* Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> [250812 11:47]:
> There is an issue with the mask declarations in linux/mm_types.h, which
> naively do (1 << bit) operations. Unfortunately this results in the 1 being
> defaulted as a signed (32-bit) integer.
> 
> When the compiler expands the MMF_INIT_MASK bitmask it comes up with:
> 
> (((1 << 2) - 1) | (((1 << 9) - 1) << 2) | (1 << 24) | (1 << 28) | (1 << 30)
> | (1 << 31))
> 
> Which overflows the signed integer to -788,527,105. Implicitly casting this
> to an unsigned integer results in sign-expansion, and thus this value
> becomes 0xffffffffd10007ff, rather than the intended 0xd10007ff.
> 
> While we're limited to a maximum of 32 bits in mm->flags, this isn't an
> issue as the remaining bits being masked will always be zero.
> 
> However, now we are moving towards having more bits in this flag, this
> becomes an issue.
> 
> Simply resolve this by using the _BITUL() helper to cast the shifted value
> to an unsigned long.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>

Reviewed-by: Liam R. Howlett <Liam.Howlett@xxxxxxxxxx>

> ---
>  include/linux/mm_types.h | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 46d3fb8935c7..38b3fa927997 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1756,7 +1756,7 @@ enum {
>   * the modes are SUID_DUMP_* defined in linux/sched/coredump.h
>   */
>  #define MMF_DUMPABLE_BITS 2
> -#define MMF_DUMPABLE_MASK ((1 << MMF_DUMPABLE_BITS) - 1)
> +#define MMF_DUMPABLE_MASK (_BITUL(MMF_DUMPABLE_BITS) - 1)
>  /* coredump filter bits */
>  #define MMF_DUMP_ANON_PRIVATE	2
>  #define MMF_DUMP_ANON_SHARED	3
> @@ -1771,13 +1771,13 @@ enum {
>  #define MMF_DUMP_FILTER_SHIFT	MMF_DUMPABLE_BITS
>  #define MMF_DUMP_FILTER_BITS	9
>  #define MMF_DUMP_FILTER_MASK \
> -	(((1 << MMF_DUMP_FILTER_BITS) - 1) << MMF_DUMP_FILTER_SHIFT)
> +	((_BITUL(MMF_DUMP_FILTER_BITS) - 1) << MMF_DUMP_FILTER_SHIFT)
>  #define MMF_DUMP_FILTER_DEFAULT \
> -	((1 << MMF_DUMP_ANON_PRIVATE) |	(1 << MMF_DUMP_ANON_SHARED) |\
> -	 (1 << MMF_DUMP_HUGETLB_PRIVATE) | MMF_DUMP_MASK_DEFAULT_ELF)
> +	(_BITUL(MMF_DUMP_ANON_PRIVATE) | _BITUL(MMF_DUMP_ANON_SHARED) | \
> +	 _BITUL(MMF_DUMP_HUGETLB_PRIVATE) | MMF_DUMP_MASK_DEFAULT_ELF)
>  
>  #ifdef CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS
> -# define MMF_DUMP_MASK_DEFAULT_ELF	(1 << MMF_DUMP_ELF_HEADERS)
> +# define MMF_DUMP_MASK_DEFAULT_ELF	_BITUL(MMF_DUMP_ELF_HEADERS)
>  #else
>  # define MMF_DUMP_MASK_DEFAULT_ELF	0
>  #endif
> @@ -1797,7 +1797,7 @@ enum {
>  #define MMF_UNSTABLE		22	/* mm is unstable for copy_from_user */
>  #define MMF_HUGE_ZERO_FOLIO	23      /* mm has ever used the global huge zero folio */
>  #define MMF_DISABLE_THP		24	/* disable THP for all VMAs */
> -#define MMF_DISABLE_THP_MASK	(1 << MMF_DISABLE_THP)
> +#define MMF_DISABLE_THP_MASK	_BITUL(MMF_DISABLE_THP)
>  #define MMF_OOM_REAP_QUEUED	25	/* mm was queued for oom_reaper */
>  #define MMF_MULTIPROCESS	26	/* mm is shared between processes */
>  /*
> @@ -1810,16 +1810,15 @@ enum {
>  #define MMF_HAS_PINNED		27	/* FOLL_PIN has run, never cleared */
>  
>  #define MMF_HAS_MDWE		28
> -#define MMF_HAS_MDWE_MASK	(1 << MMF_HAS_MDWE)
> -
> +#define MMF_HAS_MDWE_MASK	_BITUL(MMF_HAS_MDWE)
>  
>  #define MMF_HAS_MDWE_NO_INHERIT	29
>  
>  #define MMF_VM_MERGE_ANY	30
> -#define MMF_VM_MERGE_ANY_MASK	(1 << MMF_VM_MERGE_ANY)
> +#define MMF_VM_MERGE_ANY_MASK	_BITUL(MMF_VM_MERGE_ANY)
>  
>  #define MMF_TOPDOWN		31	/* mm searches top down by default */
> -#define MMF_TOPDOWN_MASK	(1 << MMF_TOPDOWN)
> +#define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
>  
>  #define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>  				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
> -- 
> 2.50.1
>