mm: kmsan: handle alloc failures in kmsan_vmap_pages_range_noflush()

Similarly to kmsan_vmap_pages_range_noflush(), kmsan_ioremap_page_range() must also properly handle allocation/mapping failures. In the case of such, it must clean up the already created metadata mappings and return an error code, so that the error can be propagated to ioremap_page_range(). Without doing so, KMSAN may silently fail to bring the metadata for the page range into a consistent state, which will result in user-visible crashes when trying to access them.

Reported-by Dipanjan Das <[email protected]>

Link https://lore.kernel.org/linux-mm/CANX2M5ZRrRA64k0hOif02TjmY9kbbO2aCBPyq79es34RXZ=cAw@mail.gmail.com/ open here

Fixes b073d7f8aee4 ("mm: kmsan: maintain KMSAN metadata for page operations")

Signed-off-by Alexander Potapenko <[email protected]>

---
v2:
-- updated patch description as requested by Andrew Morton
-- check the return value of __vmap_pages_range_noflush(), as suggested by Dipanjan Das -- return 0 from the inline version of kmsan_ioremap_page_range()
(spotted by kernel test robot <[email protected]>)

---
 include/linux/kmsan.h | 19 ++++++++-------
 mm/kmsan/hooks.c      | 55 ++++++++++++++++++++++++++++++++++++-------
 mm/vmalloc.c          |  4 ++--
 3 files changed, 59 insertions(+), 19 deletions(-)

include/linux/kmsan.h +10 -9

index c7ff3aefc5a13..30b17647ce3c7 100644
@@ -160,11 +160,12 @@ void kmsan_vunmap_range_noflush(unsigned long start, unsigned long end);
  * @page_shift:        page_shift argument passed to vmap_range_noflush().
  *
  * KMSAN creates new metadata pages for the physical pages mapped into the
- * virtual memory.
+ * virtual memory. Returns 0 on success, callers must check for non-zero return
+ * value.
  */
-void kmsan_ioremap_page_range(unsigned long addr, unsigned long end,
-                             phys_addr_t phys_addr, pgprot_t prot,
-                             unsigned int page_shift);
+int kmsan_ioremap_page_range(unsigned long addr, unsigned long end,
+                            phys_addr_t phys_addr, pgprot_t prot,
+                            unsigned int page_shift);
 
 /**
  * kmsan_iounmap_page_range() - Notify KMSAN about a iounmap_page_range() call.

@@ -296,12 +297,12 @@ static inline void kmsan_vunmap_range_noflush(unsigned long start,
 {
 }
 
-static inline void kmsan_ioremap_page_range(unsigned long start,
-                                           unsigned long end,
-                                           phys_addr_t phys_addr,
-                                           pgprot_t prot,
-                                           unsigned int page_shift)
+static inline int kmsan_ioremap_page_range(unsigned long start,
+                                          unsigned long end,
+                                          phys_addr_t phys_addr, pgprot_t prot,
+                                          unsigned int page_shift)
 {
+       return 0;
 }
 
 static inline void kmsan_iounmap_page_range(unsigned long start,

mm/kmsan/hooks.c +47 -8

index 3807502766a3e..ec0da72e65aa0 100644
@@ -148,35 +148,74 @@ void kmsan_vunmap_range_noflush(unsigned long start, unsigned long end)
  * into the virtual memory. If those physical pages already had shadow/origin,
  * those are ignored.
  */
-void kmsan_ioremap_page_range(unsigned long start, unsigned long end,
-                             phys_addr_t phys_addr, pgprot_t prot,
-                             unsigned int page_shift)
+int kmsan_ioremap_page_range(unsigned long start, unsigned long end,
+                            phys_addr_t phys_addr, pgprot_t prot,
+                            unsigned int page_shift)
 {
        gfp_t gfp_mask = GFP_KERNEL | __GFP_ZERO;
        struct page *shadow, *origin;
        unsigned long off = 0;
-       int nr;
+       int nr, err = 0, clean = 0, mapped;
 
        if (!kmsan_enabled || kmsan_in_runtime())
-               return;
+               return 0;
 
        nr = (end - start) / PAGE_SIZE;
        kmsan_enter_runtime();
-       for (int i = 0; i < nr; i++, off += PAGE_SIZE) {
+       for (int i = 0; i < nr; i++, off += PAGE_SIZE, clean = i) {
                shadow = alloc_pages(gfp_mask, 1);
                origin = alloc_pages(gfp_mask, 1);
-               __vmap_pages_range_noflush(
+               if (!shadow || !origin) {
+                       err = -ENOMEM;
+                       goto ret;
+               }
+               mapped = __vmap_pages_range_noflush(
                        vmalloc_shadow(start + off),
                        vmalloc_shadow(start + off + PAGE_SIZE), prot, &shadow,
                        PAGE_SHIFT);
-               __vmap_pages_range_noflush(
+               if (mapped) {
+                       err = mapped;
+                       goto ret;
+               }
+               shadow = NULL;
+               mapped = __vmap_pages_range_noflush(
                        vmalloc_origin(start + off),
                        vmalloc_origin(start + off + PAGE_SIZE), prot, &origin,
                        PAGE_SHIFT);
+               if (mapped) {
+                       __vunmap_range_noflush(
+                               vmalloc_shadow(start + off),
+                               vmalloc_shadow(start + off + PAGE_SIZE));
+                       err = mapped;
+                       goto ret;
+               }
+               origin = NULL;
+       }
+       /* Page mapping loop finished normally, nothing to clean up. */
+       clean = 0;
+
+ret:
+       if (clean > 0) {
+               /*
+                * Something went wrong. Clean up shadow/origin pages allocated
+                * on the last loop iteration, then delete mappings created
+                * during the previous iterations.
+                */
+               if (shadow)
+                       __free_pages(shadow, 1);
+               if (origin)
+                       __free_pages(origin, 1);
+               __vunmap_range_noflush(
+                       vmalloc_shadow(start),
+                       vmalloc_shadow(start + clean * PAGE_SIZE));
+               __vunmap_range_noflush(
+                       vmalloc_origin(start),
+                       vmalloc_origin(start + clean * PAGE_SIZE));
        }
        flush_cache_vmap(vmalloc_shadow(start), vmalloc_shadow(end));
        flush_cache_vmap(vmalloc_origin(start), vmalloc_origin(end));
        kmsan_leave_runtime();
+       return err;
 }
 
 void kmsan_iounmap_page_range(unsigned long start, unsigned long end)

mm/vmalloc.c +2 -3

index 1355d95cce1ca..31ff782d368b0 100644
@@ -313,8 +313,8 @@ int ioremap_page_range(unsigned long addr, unsigned long end,
                                 ioremap_max_page_shift);
        flush_cache_vmap(addr, end);
        if (!err)
-               kmsan_ioremap_page_range(addr, end, phys_addr, prot,
-                                        ioremap_max_page_shift);
+               err = kmsan_ioremap_page_range(addr, end, phys_addr, prot,
+                                              ioremap_max_page_shift);
        return err;
 }
 
--

2.40.0.577.gac1e443424-goog

Non-void KMSAN hooks may return error codes that indicate that KMSAN failed to reflect the changed memory state in the metadata (e.g. it could not create the necessary memory mappings). In such cases the callers should handle the errors to prevent the tool from using the inconsistent metadata in the future.

We mark non-void hooks with __must_check so that error handling is not skipped.

Signed-off-by Alexander Potapenko <[email protected]>

---
 include/linux/kmsan.h | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

include/linux/kmsan.h +22 -22

index 30b17647ce3c7..e0c23a32cdf01 100644
@@ -54,7 +54,8 @@ void __init kmsan_init_runtime(void);
  * Freed pages are either returned to buddy allocator or held back to be used
  * as metadata pages.
  */
-bool __init kmsan_memblock_free_pages(struct page *page, unsigned int order);
+bool __init __must_check kmsan_memblock_free_pages(struct page *page,
+                                                  unsigned int order);
 
 /**
  * kmsan_alloc_page() - Notify KMSAN about an alloc_pages() call.

@@ -137,9 +138,11 @@ void kmsan_kfree_large(const void *ptr);
  * vmalloc metadata address range. Returns 0 on success, callers must check
  * for non-zero return value.
  */
-int kmsan_vmap_pages_range_noflush(unsigned long start, unsigned long end,
-                                  pgprot_t prot, struct page **pages,
-                                  unsigned int page_shift);
+int __must_check kmsan_vmap_pages_range_noflush(unsigned long start,
+                                               unsigned long end,
+                                               pgprot_t prot,
+                                               struct page **pages,
+                                               unsigned int page_shift);
 
 /**
  * kmsan_vunmap_kernel_range_noflush() - Notify KMSAN about a vunmap.

@@ -163,9 +166,9 @@ void kmsan_vunmap_range_noflush(unsigned long start, unsigned long end);
  * virtual memory. Returns 0 on success, callers must check for non-zero return
  * value.
  */
-int kmsan_ioremap_page_range(unsigned long addr, unsigned long end,
-                            phys_addr_t phys_addr, pgprot_t prot,
-                            unsigned int page_shift);
+int __must_check kmsan_ioremap_page_range(unsigned long addr, unsigned long end,
+                                         phys_addr_t phys_addr, pgprot_t prot,
+                                         unsigned int page_shift);
 
 /**
  * kmsan_iounmap_page_range() - Notify KMSAN about a iounmap_page_range() call.

@@ -237,8 +240,8 @@ static inline void kmsan_init_runtime(void)
 {
 }
 
-static inline bool kmsan_memblock_free_pages(struct page *page,
-                                            unsigned int order)
+static inline bool __must_check kmsan_memblock_free_pages(struct page *page,
+                                                         unsigned int order)
 {
        return true;
 }

@@ -251,10 +254,9 @@ static inline void kmsan_task_exit(struct task_struct *task)
 {
 }
 
-static inline int kmsan_alloc_page(struct page *page, unsigned int order,
-                                  gfp_t flags)
+static inline void kmsan_alloc_page(struct page *page, unsigned int order,
+                                   gfp_t flags)
 {
-       return 0;
 }
 
 static inline void kmsan_free_page(struct page *page, unsigned int order)

@@ -283,11 +285,9 @@ static inline void kmsan_kfree_large(const void *ptr)
 {
 }
 
-static inline int kmsan_vmap_pages_range_noflush(unsigned long start,
-                                                unsigned long end,
-                                                pgprot_t prot,
-                                                struct page **pages,
-                                                unsigned int page_shift)
+static inline int __must_check kmsan_vmap_pages_range_noflush(
+       unsigned long start, unsigned long end, pgprot_t prot,
+       struct page **pages, unsigned int page_shift)
 {
        return 0;
 }

@@ -297,10 +297,11 @@ static inline void kmsan_vunmap_range_noflush(unsigned long start,
 {
 }
 
-static inline int kmsan_ioremap_page_range(unsigned long start,
-                                          unsigned long end,
-                                          phys_addr_t phys_addr, pgprot_t prot,
-                                          unsigned int page_shift)
+static inline int __must_check kmsan_ioremap_page_range(unsigned long start,
+                                                       unsigned long end,
+                                                       phys_addr_t phys_addr,
+                                                       pgprot_t prot,
+                                                       unsigned int page_shift)
 {
        return 0;
 }
--

2.40.0.577.gac1e443424-goog

--
You received this message because you are subscribed to the Google Groups "kasan-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20230413131223.4135168-3-glider%40google.com.

To prevent errors when vmap_pages_range_noflush() or __vmap_pages_range_noflush() silently fail (see the link below for an example), annotate them with __must_check so that the callers do not unconditionally assume the mapping succeeded.

Reported-by Dipanjan Das <[email protected]>

Link https://lore.kernel.org/linux-mm/CANX2M5ZRrRA64k0hOif02TjmY9kbbO2aCBPyq79es34RXZ=cAw@mail.gmail.com/ open here

Signed-off-by Alexander Potapenko <[email protected]>

---
 mm/internal.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

mm/internal.h +7 -8

index 7920a8b7982ec..a646cf7c41e8a 100644
@@ -833,20 +833,20 @@ size_t splice_folio_into_pipe(struct pipe_inode_info *pipe,
  * mm/vmalloc.c
  */
 #ifdef CONFIG_MMU
-int vmap_pages_range_noflush(unsigned long addr, unsigned long end,
-                pgprot_t prot, struct page **pages, unsigned int page_shift);
+int __must_check vmap_pages_range_noflush(unsigned long addr, unsigned long end,
+               pgprot_t prot, struct page **pages, unsigned int page_shift);
 #else
 static inline
-int vmap_pages_range_noflush(unsigned long addr, unsigned long end,
-                pgprot_t prot, struct page **pages, unsigned int page_shift)
+int __must_check vmap_pages_range_noflush(unsigned long addr, unsigned long end,
+               pgprot_t prot, struct page **pages, unsigned int page_shift)
 {
        return -EINVAL;
 }
 #endif
 
-int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
-                              pgprot_t prot, struct page **pages,
-                              unsigned int page_shift);
+int __must_check __vmap_pages_range_noflush(
+       unsigned long addr, unsigned long end, pgprot_t prot,
+       struct page **pages, unsigned int page_shift);
 
 void vunmap_range_noflush(unsigned long start, unsigned long end);
 
--

2.40.0.577.gac1e443424-goog

As reported by Dipanjan Das, when KMSAN is used together with kernel fault injection (or, generally, even without the latter), calls to kcalloc() or __vmap_pages_range_noflush() may fail, leaving the metadata mappings for the virtual mapping in an inconsistent state. When these metadata mappings are accessed later, the kernel crashes.

To address the problem, we return a non-zero error code from kmsan_vmap_pages_range_noflush() in the case of any allocation/mapping failure inside it, and make vmap_pages_range_noflush() return an error if KMSAN fails to allocate the metadata.

This patch also removes KMSAN_WARN_ON() from vmap_pages_range_noflush(), as these allocation failures are not fatal anymore.

Reported-by Dipanjan Das <[email protected]>

Link https://lore.kernel.org/linux-mm/CANX2M5ZRrRA64k0hOif02TjmY9kbbO2aCBPyq79es34RXZ=cAw@mail.gmail.com/ open here

Fixes b073d7f8aee4 ("mm: kmsan: maintain KMSAN metadata for page operations")

Signed-off-by Alexander Potapenko <[email protected]>

---
v2:
-- return 0 from the inline version of kmsan_vmap_pages_range_noflush()
(spotted by kernel test robot <[email protected]>)

---
 include/linux/kmsan.h | 20 +++++++++++---------
 mm/kmsan/shadow.c     | 27 ++++++++++++++++++---------
 mm/vmalloc.c          |  6 +++++-
 3 files changed, 34 insertions(+), 19 deletions(-)

include/linux/kmsan.h +11 -9

index e38ae3c346184..c7ff3aefc5a13 100644
@@ -134,11 +134,12 @@ void kmsan_kfree_large(const void *ptr);
  * @page_shift:        page_shift passed to vmap_range_noflush().
  *
  * KMSAN maps shadow and origin pages of @pages into contiguous ranges in
- * vmalloc metadata address range.
+ * vmalloc metadata address range. Returns 0 on success, callers must check
+ * for non-zero return value.
  */
-void kmsan_vmap_pages_range_noflush(unsigned long start, unsigned long end,
-                                   pgprot_t prot, struct page **pages,
-                                   unsigned int page_shift);
+int kmsan_vmap_pages_range_noflush(unsigned long start, unsigned long end,
+                                  pgprot_t prot, struct page **pages,
+                                  unsigned int page_shift);
 
 /**
  * kmsan_vunmap_kernel_range_noflush() - Notify KMSAN about a vunmap.

@@ -281,12 +282,13 @@ static inline void kmsan_kfree_large(const void *ptr)
 {
 }
 
-static inline void kmsan_vmap_pages_range_noflush(unsigned long start,
-                                                 unsigned long end,
-                                                 pgprot_t prot,
-                                                 struct page **pages,
-                                                 unsigned int page_shift)
+static inline int kmsan_vmap_pages_range_noflush(unsigned long start,
+                                                unsigned long end,
+                                                pgprot_t prot,
+                                                struct page **pages,
+                                                unsigned int page_shift)
 {
+       return 0;
 }
 
 static inline void kmsan_vunmap_range_noflush(unsigned long start,

mm/kmsan/shadow.c +18 -9

index a787c04e9583c..b8bb95eea5e3d 100644
@@ -216,27 +216,29 @@ void kmsan_free_page(struct page *page, unsigned int order)
        kmsan_leave_runtime();
 }
 
-void kmsan_vmap_pages_range_noflush(unsigned long start, unsigned long end,
-                                   pgprot_t prot, struct page **pages,
-                                   unsigned int page_shift)
+int kmsan_vmap_pages_range_noflush(unsigned long start, unsigned long end,
+                                  pgprot_t prot, struct page **pages,
+                                  unsigned int page_shift)
 {
        unsigned long shadow_start, origin_start, shadow_end, origin_end;
        struct page **s_pages, **o_pages;
-       int nr, mapped;
+       int nr, mapped, err = 0;
 
        if (!kmsan_enabled)
-               return;
+               return 0;
 
        shadow_start = vmalloc_meta((void *)start, KMSAN_META_SHADOW);
        shadow_end = vmalloc_meta((void *)end, KMSAN_META_SHADOW);
        if (!shadow_start)
-               return;
+               return 0;
 
        nr = (end - start) / PAGE_SIZE;
        s_pages = kcalloc(nr, sizeof(*s_pages), GFP_KERNEL);
        o_pages = kcalloc(nr, sizeof(*o_pages), GFP_KERNEL);
-       if (!s_pages || !o_pages)
+       if (!s_pages || !o_pages) {
+               err = -ENOMEM;
                goto ret;
+       }
        for (int i = 0; i < nr; i++) {
                s_pages[i] = shadow_page_for(pages[i]);
                o_pages[i] = origin_page_for(pages[i]);

@@ -249,10 +251,16 @@ void kmsan_vmap_pages_range_noflush(unsigned long start, unsigned long end,
        kmsan_enter_runtime();
        mapped = __vmap_pages_range_noflush(shadow_start, shadow_end, prot,
                                            s_pages, page_shift);
-       KMSAN_WARN_ON(mapped);
+       if (mapped) {
+               err = mapped;
+               goto ret;
+       }
        mapped = __vmap_pages_range_noflush(origin_start, origin_end, prot,
                                            o_pages, page_shift);
-       KMSAN_WARN_ON(mapped);
+       if (mapped) {
+               err = mapped;
+               goto ret;
+       }
        kmsan_leave_runtime();
        flush_tlb_kernel_range(shadow_start, shadow_end);
        flush_tlb_kernel_range(origin_start, origin_end);

@@ -262,6 +270,7 @@ void kmsan_vmap_pages_range_noflush(unsigned long start, unsigned long end,
 ret:
        kfree(s_pages);
        kfree(o_pages);
+       return err;
 }
 
 /* Allocate metadata for pages allocated at boot time. */

mm/vmalloc.c +5 -2

index a50072066221a..1355d95cce1ca 100644
@@ -605,7 +605,11 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
 int vmap_pages_range_noflush(unsigned long addr, unsigned long end,
                pgprot_t prot, struct page **pages, unsigned int page_shift)
 {
-       kmsan_vmap_pages_range_noflush(addr, end, prot, pages, page_shift);
+       int ret = kmsan_vmap_pages_range_noflush(addr, end, prot, pages,
+                                                page_shift);
+
+       if (ret)
+               return ret;
        return __vmap_pages_range_noflush(addr, end, prot, pages, page_shift);
 }
 
--

2.40.0.577.gac1e443424-goog