From: Ira Weiny <[email protected]>
kmap_to_page() is used to get the page for a virtual address which may
be kmap'ed. Unfortunately, kmap_local_page() stores mappings in a
thread local array separate from kmap(). These mappings were not
checked by the call.
Check the kmap_local_page() mappings and return the page if found.
Because it is intended to remove kmap_to_page() add a warn on once to
the kmap checks to flag potential issues early.
NOTE Due to 32bit x86 use of kmap local in iomap atmoic, KMAP_LOCAL does
not require HIGHMEM to be set. Therefore the support calls required a
new KMAP_LOCAL section to fix 0day build errors.
Cc: "Fabio M. De Francesco" <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Andrew Morton <[email protected]>
Reported-by: kernel test robot <[email protected]>
Reported-by: Al Viro <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>
---
Changes from V1:
Fix 0day build errors.
I'm still working toward getting rid of kmap_to_page.[1] But until then
this fix should be applied.
[1] https://lore.kernel.org/linux-mm/[email protected]/
---
mm/highmem.c | 42 ++++++++++++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 10 deletions(-)
diff --git a/mm/highmem.c b/mm/highmem.c
index c707d7202d5f..529987b28205 100644
--- a/mm/highmem.c
+++ b/mm/highmem.c
@@ -30,6 +30,17 @@
#include <asm/tlbflush.h>
#include <linux/vmalloc.h>
+#ifdef CONFIG_KMAP_LOCAL
+static inline int kmap_local_calc_idx(int idx)
+{
+ return idx + KM_MAX_IDX * smp_processor_id();
+}
+
+#ifndef arch_kmap_local_map_idx
+#define arch_kmap_local_map_idx(idx, pfn) kmap_local_calc_idx(idx)
+#endif
+#endif /* CONFIG_KMAP_LOCAL */
+
/*
* Virtual_count is not a pure "count".
* 0 means that it is not mapped, and has not been mapped
@@ -142,14 +153,34 @@ pte_t *pkmap_page_table;
struct page *__kmap_to_page(void *vaddr)
{
+ unsigned long base = (unsigned long) vaddr & PAGE_MASK;
+ struct kmap_ctrl *kctrl = ¤t->kmap_ctrl;
unsigned long addr = (unsigned long)vaddr;
+ int i;
- if (addr >= PKMAP_ADDR(0) && addr < PKMAP_ADDR(LAST_PKMAP)) {
+ /* kmap() mappings */
+ if (WARN_ON_ONCE(addr >= PKMAP_ADDR(0) &&
+ addr < PKMAP_ADDR(LAST_PKMAP))) {
int i = PKMAP_NR(addr);
return pte_page(pkmap_page_table[i]);
}
+ /* kmap_local_page() mappings */
+ if (WARN_ON_ONCE(base >= __fix_to_virt(FIX_KMAP_END) &&
+ base < __fix_to_virt(FIX_KMAP_BEGIN))) {
+ for (i = 0; i < kctrl->idx; i++) {
+ unsigned long base_addr;
+ int idx;
+
+ idx = arch_kmap_local_map_idx(i, pte_pfn(pteval));
+ base_addr = __fix_to_virt(FIX_KMAP_BEGIN + idx);
+
+ if (base_addr == base)
+ return pte_page(kctrl->pteval[i]);
+ }
+ }
+
return virt_to_page(vaddr);
}
EXPORT_SYMBOL(__kmap_to_page);
@@ -462,10 +493,6 @@ static inline void kmap_local_idx_pop(void)
# define arch_kmap_local_post_unmap(vaddr) do { } while (0)
#endif
-#ifndef arch_kmap_local_map_idx
-#define arch_kmap_local_map_idx(idx, pfn) kmap_local_calc_idx(idx)
-#endif
-
#ifndef arch_kmap_local_unmap_idx
#define arch_kmap_local_unmap_idx(idx, vaddr) kmap_local_calc_idx(idx)
#endif
@@ -494,11 +521,6 @@ static inline bool kmap_high_unmap_local(unsigned long vaddr)
return false;
}
-static inline int kmap_local_calc_idx(int idx)
-{
- return idx + KM_MAX_IDX * smp_processor_id();
-}
-
static pte_t *__kmap_pte;
static pte_t *kmap_get_pte(unsigned long vaddr, int idx)
--
2.37.2
On Wed, 5 Oct 2022 21:05:55 -0700 [email protected] wrote:
> kmap_to_page() is used to get the page for a virtual address which may
> be kmap'ed. Unfortunately, kmap_local_page() stores mappings in a
> thread local array separate from kmap(). These mappings were not
> checked by the call.
>
> Check the kmap_local_page() mappings and return the page if found.
>
> Because it is intended to remove kmap_to_page() add a warn on once to
> the kmap checks to flag potential issues early.
What were the user-visible runtime effects of this?
Are we able to identify a Fixes:?
Thanks.
On Thu, Oct 06, 2022 at 01:33:31PM -0700, Andrew Morton wrote:
> On Wed, 5 Oct 2022 21:05:55 -0700 [email protected] wrote:
>
> > kmap_to_page() is used to get the page for a virtual address which may
> > be kmap'ed. Unfortunately, kmap_local_page() stores mappings in a
> > thread local array separate from kmap(). These mappings were not
> > checked by the call.
> >
> > Check the kmap_local_page() mappings and return the page if found.
> >
> > Because it is intended to remove kmap_to_page() add a warn on once to
> > the kmap checks to flag potential issues early.
>
> What were the user-visible runtime effects of this?
No one actually hit a bug with this because AFAIK only one kmap() call has been
converted to kmap_local_page() which then eventually calls kmap_to_page().
https://lore.kernel.org/lkml/YzN+ZYLjK6HI1P1C@ZenIV/
However that has already been fixed by Al in that thread.
>
> Are we able to identify a Fixes:?
I suppose this could be added as a Fixes: to the original patch introducing
kmap_local_page()? But one could argue that kmap_to_page() was only intended
to support kmap() addresses because it does not work with kmap_atomic()
addresses either.
I'm proposing this as a stop gap to ensure that work can continue on converting
kmap() uses to kmap_local_page() without fear of causing breakage while
simultaneously we evaluate and hopefully remove kmap_to_page() as well.
Ira