[PATCH 3/5] hibernate: handle potential errors in hibernate_{map,unmap}_page()

["Mike Rapoport (Microsoft)" <rppt@kernel.org>]

When safe_copy_page() had to map/unmap pages only because of
debug_pagealloc() there could be no errors in the kernel page table
updates.

However, with the increasing desire to remove pages from the direct map
this assumption becomes a real stretch.

Properly handle errors in hibernate_map_page() and hibernate_unmap_page()
and propagate that error along the page copying path.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 kernel/power/snapshot.c | 102 +++++++++++++++++++++++++++++-------------------
 1 file changed, 62 insertions(+), 40 deletions(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index e732bf6389e4c..52ef0599c2076 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -79,37 +79,30 @@ static inline int hibernate_restore_unprotect_page(void *page_address) {return 0
 #endif /* CONFIG_STRICT_KERNEL_RWX  && CONFIG_ARCH_HAS_SET_MEMORY */
 
 
-/*
- * The calls to set_direct_map_*() should not fail because remapping a page
- * here means that we only update protection bits in an existing PTE.
- * It is still worth to have a warning here if something changes and this
- * will no longer be the case.
- */
-static inline void hibernate_map_page(struct page *page)
+static inline int hibernate_map_page(struct page *page)
 {
 	if (IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) {
-		int ret = set_direct_map_default_noflush(page, 1);
-
-		if (ret)
-			pr_warn_once("Failed to remap page\n");
+		return set_direct_map_default_noflush(page, 1);
 	} else {
 		debug_pagealloc_map_pages(page, 1);
+		return 0;
 	}
 }
 
-static inline void hibernate_unmap_page(struct page *page)
+static inline int hibernate_unmap_page(struct page *page)
 {
 	if (IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) {
 		unsigned long addr = (unsigned long)page_address(page);
 		int ret  = set_direct_map_invalid_noflush(page, 1);
 
 		if (ret)
-			pr_warn_once("Failed to remap page\n");
+			return ret;
 
 		flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
 	} else {
 		debug_pagealloc_unmap_pages(page, 1);
 	}
+	return 0;
 }
 
 static int swsusp_page_is_free(struct page *page);
@@ -1449,21 +1442,25 @@ static inline bool do_copy_page(long *dst, long *src)
  * Check if the page we are going to copy is marked as present in the kernel
  * page tables. This always is the case if CONFIG_DEBUG_PAGEALLOC or
  * CONFIG_ARCH_HAS_SET_DIRECT_MAP is not set. In that case kernel_page_present()
- * always returns 'true'. Returns true if the page was entirely composed of
- * zeros, otherwise it will return false.
+ * always returns 'true'.
+ * Sets @zeros_only to true if the page was entirely composed of zeros.
+ *
+ * Returns 0 on success, a negative error code on failure.
  */
-static bool safe_copy_page(void *dst, struct page *s_page)
+static int safe_copy_page(void *dst, struct page *s_page, bool *zeros_only)
 {
-	bool zeros_only;
+	int err;
 
 	if (kernel_page_present(s_page)) {
-		zeros_only = do_copy_page(dst, page_address(s_page));
-	} else {
-		hibernate_map_page(s_page);
-		zeros_only = do_copy_page(dst, page_address(s_page));
-		hibernate_unmap_page(s_page);
+		*zeros_only = do_copy_page(dst, page_address(s_page));
+		return 0;
 	}
-	return zeros_only;
+
+	err = hibernate_map_page(s_page);
+	if (err)
+		return err;
+	*zeros_only = do_copy_page(dst, page_address(s_page));
+	return hibernate_unmap_page(s_page);
 }
 
 #ifdef CONFIG_HIGHMEM
@@ -1473,18 +1470,19 @@ static inline struct page *page_is_saveable(struct zone *zone, unsigned long pfn
 		saveable_highmem_page(zone, pfn) : saveable_page(zone, pfn);
 }
 
-static bool copy_data_page(unsigned long dst_pfn, unsigned long src_pfn)
+static int copy_data_page(unsigned long dst_pfn, unsigned long src_pfn,
+			  bool *zeros_only)
 {
 	struct page *s_page, *d_page;
 	void *src, *dst;
-	bool zeros_only;
+	int err = 0;
 
 	s_page = pfn_to_page(src_pfn);
 	d_page = pfn_to_page(dst_pfn);
 	if (PageHighMem(s_page)) {
 		src = kmap_local_page(s_page);
 		dst = kmap_local_page(d_page);
-		zeros_only = do_copy_page(dst, src);
+		*zeros_only = do_copy_page(dst, src);
 		kunmap_local(dst);
 		kunmap_local(src);
 	} else {
@@ -1493,23 +1491,29 @@ static bool copy_data_page(unsigned long dst_pfn, unsigned long src_pfn)
 			 * The page pointed to by src may contain some kernel
 			 * data modified by kmap_atomic()
 			 */
-			zeros_only = safe_copy_page(buffer, s_page);
+			err = safe_copy_page(buffer, s_page, zeros_only);
+			if (err)
+				goto out;
 			dst = kmap_local_page(d_page);
 			copy_page(dst, buffer);
 			kunmap_local(dst);
 		} else {
-			zeros_only = safe_copy_page(page_address(d_page), s_page);
+			err = safe_copy_page(page_address(d_page), s_page,
+					     zeros_only);
 		}
 	}
-	return zeros_only;
+out:
+	return err;
+
 }
 #else
 #define page_is_saveable(zone, pfn)	saveable_page(zone, pfn)
 
-static inline int copy_data_page(unsigned long dst_pfn, unsigned long src_pfn)
+static inline int copy_data_page(unsigned long dst_pfn, unsigned long src_pfn,
+				 bool *zeros_only)
 {
 	return safe_copy_page(page_address(pfn_to_page(dst_pfn)),
-				pfn_to_page(src_pfn));
+				pfn_to_page(src_pfn), zeros_only);
 }
 #endif /* CONFIG_HIGHMEM */
 
@@ -1517,15 +1521,20 @@ static inline int copy_data_page(unsigned long dst_pfn, unsigned long src_pfn)
  * Copy data pages will copy all pages into pages pulled from the copy_bm.
  * If a page was entirely filled with zeros it will be marked in the zero_bm.
  *
- * Returns the number of pages copied.
+ * Sets @copied_pages to the number of pages copied.
+ *
+ * Returns 0 on success, a negative error code on failure.
  */
-static unsigned long copy_data_pages(struct memory_bitmap *copy_bm,
-			    struct memory_bitmap *orig_bm,
-			    struct memory_bitmap *zero_bm)
+static int copy_data_pages(struct memory_bitmap *copy_bm,
+			   struct memory_bitmap *orig_bm,
+			   struct memory_bitmap *zero_bm,
+			   unsigned int *copied_pages)
 {
-	unsigned long copied_pages = 0;
+	unsigned long nr_pages = 0;
 	struct zone *zone;
 	unsigned long pfn, copy_pfn;
+	bool zeros_only;
+	int err;
 
 	for_each_populated_zone(zone) {
 		unsigned long max_zone_pfn;
@@ -1543,15 +1552,21 @@ static unsigned long copy_data_pages(struct memory_bitmap *copy_bm,
 		pfn = memory_bm_next_pfn(orig_bm);
 		if (unlikely(pfn == BM_END_OF_MAP))
 			break;
-		if (copy_data_page(copy_pfn, pfn)) {
+		err = copy_data_page(copy_pfn, pfn, &zeros_only);
+		if (err)
+			return err;
+
+		if (zeros_only) {
 			memory_bm_set_bit(zero_bm, pfn);
 			/* Use this copy_pfn for a page that is not full of zeros */
 			continue;
 		}
-		copied_pages++;
+		nr_pages++;
 		copy_pfn = memory_bm_next_pfn(copy_bm);
 	}
-	return copied_pages;
+
+	*copied_pages = nr_pages;
+	return 0;
 }
 
 /* Total number of image pages */
@@ -2112,6 +2127,7 @@ static int swsusp_alloc(struct memory_bitmap *copy_bm,
 asmlinkage __visible int swsusp_save(void)
 {
 	unsigned int nr_pages, nr_highmem;
+	int err;
 
 	pm_deferred_pr_dbg("Creating image\n");
 
@@ -2133,7 +2149,9 @@ asmlinkage __visible int swsusp_save(void)
 	 * Kill them.
 	 */
 	drain_local_pages(NULL);
-	nr_copy_pages = copy_data_pages(&copy_bm, &orig_bm, &zero_bm);
+	err = copy_data_pages(&copy_bm, &orig_bm, &zero_bm, &nr_copy_pages);
+	if (err)
+		goto err_swsusp_free;
 
 	/*
 	 * End of critical section. From now on, we can write to memory,
@@ -2149,6 +2167,10 @@ asmlinkage __visible int swsusp_save(void)
 			   nr_copy_pages, nr_zero_pages);
 
 	return 0;
+
+err_swsusp_free:
+	swsusp_free();
+	return err;
 }
 
 #ifndef CONFIG_ARCH_HIBERNATION_HEADER

-- 
2.53.0