PM: hibernate: Don't ignore return from set_memory_ro()

set_memory_ro() and set_memory_rw() can fail, leaving memory
unprotected.

Take the returned value into account and abort in case of
failure.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

diff --git a/kernel/power/power.h b/kernel/power/power.h
index 518349272848fa404e1bccd85cd3f700f399ab09..de0e6b1077f231dc97896e31b7ce9e8204583641 100644 (file)
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -153,7 +153,7 @@ extern unsigned int snapshot_additional_pages(struct zone *zone);
 extern unsigned long snapshot_get_image_size(void);
 extern int snapshot_read_next(struct snapshot_handle *handle);
 extern int snapshot_write_next(struct snapshot_handle *handle);
-extern void snapshot_write_finalize(struct snapshot_handle *handle);
+int snapshot_write_finalize(struct snapshot_handle *handle);
 extern int snapshot_image_loaded(struct snapshot_handle *handle);
 
 extern bool hibernate_acquire(void);

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 5c96ff067c64b57e527f65a6441527f60ff74054..405eddbda4fc5728752ea580cabff4d71a2563e5 100644 (file)
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -58,22 +58,24 @@ static inline void hibernate_restore_protection_end(void)
        hibernate_restore_protection_active = false;
 }
 
-static inline void hibernate_restore_protect_page(void *page_address)
+static inline int __must_check hibernate_restore_protect_page(void *page_address)
 {
        if (hibernate_restore_protection_active)
-               set_memory_ro((unsigned long)page_address, 1);
+               return set_memory_ro((unsigned long)page_address, 1);
+       return 0;
 }
 
-static inline void hibernate_restore_unprotect_page(void *page_address)
+static inline int hibernate_restore_unprotect_page(void *page_address)
 {
        if (hibernate_restore_protection_active)
-               set_memory_rw((unsigned long)page_address, 1);
+               return set_memory_rw((unsigned long)page_address, 1);
+       return 0;
 }
 #else
 static inline void hibernate_restore_protection_begin(void) {}
 static inline void hibernate_restore_protection_end(void) {}
-static inline void hibernate_restore_protect_page(void *page_address) {}
-static inline void hibernate_restore_unprotect_page(void *page_address) {}
+static inline int __must_check hibernate_restore_protect_page(void *page_address) {return 0; }
+static inline int hibernate_restore_unprotect_page(void *page_address) {return 0; }
 #endif /* CONFIG_STRICT_KERNEL_RWX  && CONFIG_ARCH_HAS_SET_MEMORY */
 
 
@@ -2832,7 +2834,9 @@ next:
                }
        } else {
                copy_last_highmem_page();
-               hibernate_restore_protect_page(handle->buffer);
+               error = hibernate_restore_protect_page(handle->buffer);
+               if (error)
+                       return error;
                handle->buffer = get_buffer(&orig_bm, &ca);
                if (IS_ERR(handle->buffer))
                        return PTR_ERR(handle->buffer);
@@ -2858,15 +2862,18 @@ next:
  * stored in highmem.  Additionally, it recycles bitmap memory that's not
  * necessary any more.
  */
-void snapshot_write_finalize(struct snapshot_handle *handle)
+int snapshot_write_finalize(struct snapshot_handle *handle)
 {
+       int error;
+
        copy_last_highmem_page();
-       hibernate_restore_protect_page(handle->buffer);
+       error = hibernate_restore_protect_page(handle->buffer);
        /* Do that only if we have loaded the image entirely */
        if (handle->cur > 1 && handle->cur > nr_meta_pages + nr_copy_pages + nr_zero_pages) {
                memory_bm_recycle(&orig_bm);
                free_highmem_data();
        }
+       return error;
 }
 
 int snapshot_image_loaded(struct snapshot_handle *handle)

diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 6513035f2f7f3dc191e51eb48c4e0bd2b69d396a..364342cc7f2d7cd9e8e06626a0c31e09a3cdbcd2 100644 (file)
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -1134,8 +1134,8 @@ static int load_image(struct swap_map_handle *handle,
                ret = err2;
        if (!ret) {
                pr_info("Image loading done\n");
-               snapshot_write_finalize(snapshot);
-               if (!snapshot_image_loaded(snapshot))
+               ret = snapshot_write_finalize(snapshot);
+               if (!ret && !snapshot_image_loaded(snapshot))
                        ret = -ENODATA;
        }
        swsusp_show_speed(start, stop, nr_to_read, "Read");
@@ -1486,8 +1486,8 @@ out_finish:
        stop = ktime_get();
        if (!ret) {
                pr_info("Image loading done\n");
-               snapshot_write_finalize(snapshot);
-               if (!snapshot_image_loaded(snapshot))
+               ret = snapshot_write_finalize(snapshot);
+               if (!ret && !snapshot_image_loaded(snapshot))
                        ret = -ENODATA;
                if (!ret) {
                        if (swsusp_header->flags & SF_CRC32_MODE) {

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 3a4e70366f354c0167c6630c3063af8ffa26bfbf..3aa41ba22129d8d702b5102cdea235c976fcb2ac 100644 (file)
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -317,7 +317,9 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
                break;
 
        case SNAPSHOT_ATOMIC_RESTORE:
-               snapshot_write_finalize(&data->handle);
+               error = snapshot_write_finalize(&data->handle);
+               if (error)
+                       break;
                if (data->mode != O_WRONLY || !data->frozen ||
                    !snapshot_image_loaded(&data->handle)) {
                        error = -EPERM;