| 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
| ||
|
Message-ID: <a6cd422d-7c44-4a4a-bb27-ca0e2a93aa68@lucifer.local>
Date: Mon, 1 Sep 2025 15:51:46 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Max Kellermann <max.kellermann@...os.com>
Cc: akpm@...ux-foundation.org, david@...hat.com, axelrasmussen@...gle.com,
yuanchu@...gle.com, willy@...radead.org, hughd@...gle.com,
mhocko@...e.com, linux-kernel@...r.kernel.org, linux-mm@...ck.org,
Liam.Howlett@...cle.com, vbabka@...e.cz, rppt@...nel.org,
surenb@...gle.com, vishal.moola@...il.com, linux@...linux.org.uk,
James.Bottomley@...senpartnership.com, deller@....de,
agordeev@...ux.ibm.com, gerald.schaefer@...ux.ibm.com,
hca@...ux.ibm.com, gor@...ux.ibm.com, borntraeger@...ux.ibm.com,
svens@...ux.ibm.com, davem@...emloft.net, andreas@...sler.com,
dave.hansen@...ux.intel.com, luto@...nel.org, peterz@...radead.org,
tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, x86@...nel.org,
hpa@...or.com, chris@...kel.net, jcmvbkbc@...il.com,
viro@...iv.linux.org.uk, brauner@...nel.org, jack@...e.cz,
weixugc@...gle.com, baolin.wang@...ux.alibaba.com, rientjes@...gle.com,
shakeel.butt@...ux.dev, thuth@...hat.com, broonie@...nel.org,
osalvador@...e.de, jfalempe@...hat.com, mpe@...erman.id.au,
nysal@...ux.ibm.com, linux-arm-kernel@...ts.infradead.org,
linux-parisc@...r.kernel.org, linux-s390@...r.kernel.org,
sparclinux@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v5 12/12] mm: constify highmem related functions for
improved const-correctness
On Mon, Sep 01, 2025 at 02:30:28PM +0200, Max Kellermann wrote:
> Lots of functions in mm/highmem.c do not write to the given pointers
> and do not call functions that take non-const pointers and can
> therefore be constified.
>
> This includes functions like kunmap() which might be implemented in a
> way that writes to the pointer (e.g. to update reference counters or
> mapping fields), but currently are not.
>
> kmap() on the other hand cannot be made const because it calls
> set_page_address() which is non-const in some
> architectures/configurations.
>
> Signed-off-by: Max Kellermann <max.kellermann@...os.com>
Some inconsitencies on const vs. double-const here.
We need to figure this out :)
Since const <type> *val will pretty well work in all circumstances, I again
think we ought to just do that instead of double-const.
Thanks
> ---
> arch/arm/include/asm/highmem.h | 6 ++---
> arch/xtensa/include/asm/highmem.h | 2 +-
> include/linux/highmem-internal.h | 44 +++++++++++++++++--------------
> include/linux/highmem.h | 8 +++---
> mm/highmem.c | 10 +++----
> 5 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/arch/arm/include/asm/highmem.h b/arch/arm/include/asm/highmem.h
> index b4b66220952d..023be74298f3 100644
> --- a/arch/arm/include/asm/highmem.h
> +++ b/arch/arm/include/asm/highmem.h
> @@ -46,9 +46,9 @@ extern pte_t *pkmap_page_table;
> #endif
>
> #ifdef ARCH_NEEDS_KMAP_HIGH_GET
> -extern void *kmap_high_get(struct page *page);
> +extern void *kmap_high_get(const struct page *page);
>
> -static inline void *arch_kmap_local_high_get(struct page *page)
> +static inline void *arch_kmap_local_high_get(const struct page *page)
> {
> if (IS_ENABLED(CONFIG_DEBUG_HIGHMEM) && !cache_is_vivt())
> return NULL;
> @@ -57,7 +57,7 @@ static inline void *arch_kmap_local_high_get(struct page *page)
> #define arch_kmap_local_high_get arch_kmap_local_high_get
>
> #else /* ARCH_NEEDS_KMAP_HIGH_GET */
> -static inline void *kmap_high_get(struct page *page)
> +static inline void *kmap_high_get(const struct page *const page)
> {
> return NULL;
> }
> diff --git a/arch/xtensa/include/asm/highmem.h b/arch/xtensa/include/asm/highmem.h
> index 34b8b620e7f1..473b622b863b 100644
> --- a/arch/xtensa/include/asm/highmem.h
> +++ b/arch/xtensa/include/asm/highmem.h
> @@ -29,7 +29,7 @@
>
> #if DCACHE_WAY_SIZE > PAGE_SIZE
> #define get_pkmap_color get_pkmap_color
> -static inline int get_pkmap_color(struct page *page)
> +static inline int get_pkmap_color(const struct page *const page)
> {
> return DCACHE_ALIAS(page_to_phys(page));
> }
> diff --git a/include/linux/highmem-internal.h b/include/linux/highmem-internal.h
> index 36053c3d6d64..442d0efea5c7 100644
> --- a/include/linux/highmem-internal.h
> +++ b/include/linux/highmem-internal.h
> @@ -7,7 +7,7 @@
> */
> #ifdef CONFIG_KMAP_LOCAL
> void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot);
> -void *__kmap_local_page_prot(struct page *page, pgprot_t prot);
> +void *__kmap_local_page_prot(const struct page *page, pgprot_t prot);
> void kunmap_local_indexed(const void *vaddr);
> void kmap_local_fork(struct task_struct *tsk);
> void __kmap_local_sched_out(void);
> @@ -33,11 +33,11 @@ static inline void kmap_flush_tlb(unsigned long addr) { }
> #endif
>
> void *kmap_high(struct page *page);
> -void kunmap_high(struct page *page);
> +void kunmap_high(const struct page *page);
> void __kmap_flush_unused(void);
> struct page *__kmap_to_page(void *addr);
>
> -static inline void *kmap(struct page *page)
> +static inline void *kmap(struct page *const page)
> {
> void *addr;
>
> @@ -50,7 +50,7 @@ static inline void *kmap(struct page *page)
> return addr;
> }
>
> -static inline void kunmap(struct page *page)
> +static inline void kunmap(const struct page *const page)
> {
> might_sleep();
> if (!PageHighMem(page))
> @@ -68,12 +68,12 @@ static inline void kmap_flush_unused(void)
> __kmap_flush_unused();
> }
>
> -static inline void *kmap_local_page(struct page *page)
> +static inline void *kmap_local_page(const struct page *const page)
> {
> return __kmap_local_page_prot(page, kmap_prot);
> }
>
> -static inline void *kmap_local_page_try_from_panic(struct page *page)
> +static inline void *kmap_local_page_try_from_panic(const struct page *const page)
> {
> if (!PageHighMem(page))
> return page_address(page);
> @@ -81,13 +81,15 @@ static inline void *kmap_local_page_try_from_panic(struct page *page)
> return NULL;
> }
>
> -static inline void *kmap_local_folio(struct folio *folio, size_t offset)
> +static inline void *kmap_local_folio(const struct folio *const folio,
> + const size_t offset)
> {
> - struct page *page = folio_page(folio, offset / PAGE_SIZE);
> + const struct page *page = folio_page(folio, offset / PAGE_SIZE);
> return __kmap_local_page_prot(page, kmap_prot) + offset % PAGE_SIZE;
> }
>
> -static inline void *kmap_local_page_prot(struct page *page, pgprot_t prot)
> +static inline void *kmap_local_page_prot(const struct page *const page,
> + const pgprot_t prot)
> {
> return __kmap_local_page_prot(page, prot);
> }
> @@ -102,7 +104,7 @@ static inline void __kunmap_local(const void *vaddr)
> kunmap_local_indexed(vaddr);
> }
>
> -static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
> +static inline void *kmap_atomic_prot(const struct page *const page, const pgprot_t prot)
> {
> if (IS_ENABLED(CONFIG_PREEMPT_RT))
> migrate_disable();
> @@ -113,7 +115,7 @@ static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
> return __kmap_local_page_prot(page, prot);
> }
>
> -static inline void *kmap_atomic(struct page *page)
> +static inline void *kmap_atomic(const struct page *const page)
> {
> return kmap_atomic_prot(page, kmap_prot);
> }
> @@ -167,38 +169,40 @@ static inline struct page *kmap_to_page(void *addr)
> return virt_to_page(addr);
> }
>
> -static inline void *kmap(struct page *page)
> +static inline void *kmap(struct page *const page)
> {
> might_sleep();
> return page_address(page);
> }
>
> -static inline void kunmap_high(struct page *page) { }
> +static inline void kunmap_high(const struct page *const page) { }
> static inline void kmap_flush_unused(void) { }
>
> -static inline void kunmap(struct page *page)
> +static inline void kunmap(const struct page *const page)
> {
> #ifdef ARCH_HAS_FLUSH_ON_KUNMAP
> kunmap_flush_on_unmap(page_address(page));
> #endif
> }
>
> -static inline void *kmap_local_page(struct page *page)
> +static inline void *kmap_local_page(const struct page *const page)
> {
> return page_address(page);
> }
>
> -static inline void *kmap_local_page_try_from_panic(struct page *page)
> +static inline void *kmap_local_page_try_from_panic(const struct page *const page)
> {
> return page_address(page);
> }
>
> -static inline void *kmap_local_folio(struct folio *folio, size_t offset)
> +static inline void *kmap_local_folio(const struct folio *const folio,
> + const size_t offset)
> {
> return folio_address(folio) + offset;
> }
>
> -static inline void *kmap_local_page_prot(struct page *page, pgprot_t prot)
> +static inline void *kmap_local_page_prot(const struct page *const page,
> + const pgprot_t prot)
> {
> return kmap_local_page(page);
> }
> @@ -215,7 +219,7 @@ static inline void __kunmap_local(const void *addr)
> #endif
> }
>
> -static inline void *kmap_atomic(struct page *page)
> +static inline void *kmap_atomic(const struct page *const page)
> {
> if (IS_ENABLED(CONFIG_PREEMPT_RT))
> migrate_disable();
> @@ -225,7 +229,7 @@ static inline void *kmap_atomic(struct page *page)
> return page_address(page);
> }
>
> -static inline void *kmap_atomic_prot(struct page *page, pgprot_t prot)
> +static inline void *kmap_atomic_prot(const struct page *const page, const pgprot_t prot)
> {
> return kmap_atomic(page);
> }
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index 6234f316468c..105cc4c00cc3 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -43,7 +43,7 @@ static inline void *kmap(struct page *page);
> * Counterpart to kmap(). A NOOP for CONFIG_HIGHMEM=n and for mappings of
> * pages in the low memory area.
> */
> -static inline void kunmap(struct page *page);
> +static inline void kunmap(const struct page *page);
>
> /**
> * kmap_to_page - Get the page for a kmap'ed address
> @@ -93,7 +93,7 @@ static inline void kmap_flush_unused(void);
> * disabling migration in order to keep the virtual address stable across
> * preemption. No caller of kmap_local_page() can rely on this side effect.
> */
> -static inline void *kmap_local_page(struct page *page);
> +static inline void *kmap_local_page(const struct page *page);
>
> /**
> * kmap_local_folio - Map a page in this folio for temporary usage
> @@ -129,7 +129,7 @@ static inline void *kmap_local_page(struct page *page);
> * Context: Can be invoked from any context.
> * Return: The virtual address of @offset.
> */
> -static inline void *kmap_local_folio(struct folio *folio, size_t offset);
> +static inline void *kmap_local_folio(const struct folio *folio, size_t offset);
>
> /**
> * kmap_atomic - Atomically map a page for temporary usage - Deprecated!
> @@ -176,7 +176,7 @@ static inline void *kmap_local_folio(struct folio *folio, size_t offset);
> * kunmap_atomic(vaddr2);
> * kunmap_atomic(vaddr1);
> */
> -static inline void *kmap_atomic(struct page *page);
> +static inline void *kmap_atomic(const struct page *page);
>
> /* Highmem related interfaces for management code */
> static inline unsigned long nr_free_highpages(void);
> diff --git a/mm/highmem.c b/mm/highmem.c
> index ef3189b36cad..93fa505fcb98 100644
> --- a/mm/highmem.c
> +++ b/mm/highmem.c
> @@ -61,7 +61,7 @@ static inline int kmap_local_calc_idx(int idx)
> /*
> * Determine color of virtual address where the page should be mapped.
> */
> -static inline unsigned int get_pkmap_color(struct page *page)
> +static inline unsigned int get_pkmap_color(const struct page *const page)
> {
> return 0;
> }
> @@ -334,7 +334,7 @@ EXPORT_SYMBOL(kmap_high);
> *
> * This can be called from any context.
> */
> -void *kmap_high_get(struct page *page)
> +void *kmap_high_get(const struct page *const page)
> {
> unsigned long vaddr, flags;
>
> @@ -356,7 +356,7 @@ void *kmap_high_get(struct page *page)
> * If ARCH_NEEDS_KMAP_HIGH_GET is not defined then this may be called
> * only from user context.
> */
> -void kunmap_high(struct page *page)
> +void kunmap_high(const struct page *const page)
> {
> unsigned long vaddr;
> unsigned long nr;
> @@ -508,7 +508,7 @@ static inline void kmap_local_idx_pop(void)
> #endif
>
> #ifndef arch_kmap_local_high_get
> -static inline void *arch_kmap_local_high_get(struct page *page)
> +static inline void *arch_kmap_local_high_get(const struct page *const page)
> {
> return NULL;
> }
> @@ -572,7 +572,7 @@ void *__kmap_local_pfn_prot(unsigned long pfn, pgprot_t prot)
> }
> EXPORT_SYMBOL_GPL(__kmap_local_pfn_prot);
>
> -void *__kmap_local_page_prot(struct page *page, pgprot_t prot)
> +void *__kmap_local_page_prot(const struct page *const page, const pgprot_t prot)
> {
> void *kmap;
>
> --
> 2.47.2
>
Powered by blists - more mailing lists