linux-kernel - Re: [PATCH 2/2] mm/mm.h: Write folio->_flags

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50f02ce2-85f8-4dda-b3c4-ad3f20f8b9a2@amd.com>
Date: Wed, 12 Feb 2025 11:10:21 +0530
From: Shivank Garg <shivankg@....com>
To: Liu Ye <liuye@...inos.cn>, brauner@...nel.org, dhowells@...hat.com,
 akpm@...ux-foundation.org
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 2/2] mm/mm.h: Write folio->_flags_1 & 0xff as a macro
 definition

On 2/12/2025 8:28 AM, Liu Ye wrote:
> There are multiple locations in mm.h where (folio->_flags_1 & 0xff) is
> used. Write it as a macro definition to improve the readability and
> maintainability of the code.
> 
> Signed-off-by: Liu Ye <liuye@...inos.cn>
> ---
>  include/linux/mm.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7b1068ddcbb7..750e75f45557 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1098,6 +1098,8 @@ int vma_is_stack_for_current(struct vm_area_struct *vma);
>  struct mmu_gather;
>  struct inode;
>  
> +#define FOLIO_ORDER(folio) ((folio)->_flags_1 & 0xff)

This folio order calculation is only valid for !large folios.
When it's a single page (not a large folio), the memory is interpreted as struct page.

struct folio {
...
        union {
                struct {
                        unsigned long _flags_1;
                        unsigned long _head_1;
        /* public: */
                        atomic_t _large_mapcount;
                        atomic_t _entire_mapcount;
                        atomic_t _nr_pages_mapped;
                        atomic_t _pincount;
#ifdef CONFIG_64BIT
                        unsigned int _folio_nr_pages;
#endif
        /* private: the union with struct page is transitional */
                };
                struct page __page_1;
        };
...
}

I feel this to be risky, considering someone may directly use FOLIO_ORDER() macro
without folio_test_large() check.

Correct macro should look like:

#define FOLIO_ORDER(folio) (folio_test_large(folio) ? ((folio)->_flags_1 & 0xff) : 0)


Thanks,
Shivank
> +
>  /*
>   * compound_order() can be called without holding a reference, which means
>   * that niceties like page_folio() don't work.  These callers should be
> @@ -1111,7 +1113,7 @@ static inline unsigned int compound_order(struct page *page)
>  
>  	if (!test_bit(PG_head, &folio->flags))
>  		return 0;
> -	return folio->_flags_1 & 0xff;
> +	return FOLIO_ORDER(folio);
>  }
>  
>  /**
> @@ -1127,7 +1129,7 @@ static inline unsigned int folio_order(const struct folio *folio)
>  {
>  	if (!folio_test_large(folio))
>  		return 0;
> -	return folio->_flags_1 & 0xff;
> +	return FOLIO_ORDER(folio);
>  }
>  
>  #include <linux/huge_mm.h>
> @@ -2061,7 +2063,7 @@ static inline long folio_nr_pages(const struct folio *folio)
>  #ifdef CONFIG_64BIT
>  	return folio->_folio_nr_pages;
>  #else
> -	return 1L << (folio->_flags_1 & 0xff);
> +	return 1L << FOLIO_ORDER(folio);
>  #endif
>  }
>  
> @@ -2086,7 +2088,7 @@ static inline unsigned long compound_nr(struct page *page)
>  #ifdef CONFIG_64BIT
>  	return folio->_folio_nr_pages;
>  #else
> -	return 1L << (folio->_flags_1 & 0xff);
> +	return 1L << FOLIO_ORDER(folio);
>  #endif
>  }
>