On Fri, Feb 19, 2021 at 5:36 PM Matthew Garrett
<matthewgarrett@xxxxxxxxxx> wrote:
>
> Calculate and store a cryptographically secure hash of the hibernation
> image if SF_VERIFY_IMAGE is set in the hibernation flags. This allows
> detection of a corrupt image, but has the disadvantage that it requires
> the blocks be read in in linear order.
>
> Signed-off-by: Matthew Garrett <mjg59@xxxxxxxxxx>
> ---
> kernel/power/power.h | 1 +
> kernel/power/swap.c | 131 +++++++++++++++++++++++++++++++++++--------
> 2 files changed, 110 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 778bf431ec02..b8e00b9dcee8 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -168,6 +168,7 @@ extern int swsusp_swap_in_use(void);
> #define SF_PLATFORM_MODE 1
> #define SF_NOCOMPRESS_MODE 2
> #define SF_CRC32_MODE 4
> +#define SF_VERIFY_IMAGE 8
>
> /* kernel/power/hibernate.c */
> extern int swsusp_check(void);
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 72e33054a2e1..a13241a20567 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -31,6 +31,8 @@
> #include <linux/kthread.h>
> #include <linux/crc32.h>
> #include <linux/ktime.h>
> +#include <crypto/hash.h>
> +#include <crypto/sha2.h>
>
> #include "power.h"
>
> @@ -95,17 +97,20 @@ struct swap_map_page_list {
> struct swap_map_handle {
> struct swap_map_page *cur;
> struct swap_map_page_list *maps;
> + struct shash_desc *desc;
> sector_t cur_swap;
> sector_t first_sector;
> unsigned int k;
> unsigned long reqd_free_pages;
> u32 crc32;
> + u8 digest[SHA256_DIGEST_SIZE];
> };
>
> struct swsusp_header {
> char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
> - sizeof(u32)];
> + sizeof(u32) - SHA256_DIGEST_SIZE];
> u32 crc32;
> + u8 digest[SHA256_DIGEST_SIZE];
> sector_t image;
> unsigned int flags; /* Flags to pass to the "boot" kernel */
> char orig_sig[10];
> @@ -305,6 +310,9 @@ static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
> * We are relying on the behavior of blk_plug that a thread with
> * a plug will flush the plug list before sleeping.
> */
> + if (!hb)
> + return 0;
> +
> wait_event(hb->wait, atomic_read(&hb->count) == 0);
> return blk_status_to_errno(hb->error);
> }
> @@ -327,6 +335,8 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
> swsusp_header->flags = flags;
> if (flags & SF_CRC32_MODE)
> swsusp_header->crc32 = handle->crc32;
> + memcpy(swsusp_header->digest, handle->digest,
> + SHA256_DIGEST_SIZE);
> error = hib_submit_io(REQ_OP_WRITE, REQ_SYNC,
> swsusp_resume_block, swsusp_header, NULL);
> } else {
> @@ -417,6 +427,7 @@ static void release_swap_writer(struct swap_map_handle *handle)
> static int get_swap_writer(struct swap_map_handle *handle)
> {
> int ret;
> + struct crypto_shash *tfm;
>
> ret = swsusp_swap_check();
> if (ret) {
> @@ -437,7 +448,28 @@ static int get_swap_writer(struct swap_map_handle *handle)
> handle->k = 0;
> handle->reqd_free_pages = reqd_free_pages();
> handle->first_sector = handle->cur_swap;
> +
> + tfm = crypto_alloc_shash("sha256", 0, 0);
> + if (IS_ERR(tfm)) {
> + ret = -EINVAL;
> + goto err_rel;
> + }
> + handle->desc = kmalloc(sizeof(struct shash_desc) +
> + crypto_shash_descsize(tfm), GFP_KERNEL);
> + if (!handle->desc) {
> + ret = -ENOMEM;
> + goto err_rel;
> + }
> +
> + handle->desc->tfm = tfm;
> +
> + ret = crypto_shash_init(handle->desc);
> + if (ret != 0)
> + goto err_free;
> +
> return 0;
> +err_free:
> + kfree(handle->desc);
> err_rel:
> release_swap_writer(handle);
> err_close:
> @@ -446,7 +478,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
> }
>
> static int swap_write_page(struct swap_map_handle *handle, void *buf,
> - struct hib_bio_batch *hb)
> + struct hib_bio_batch *hb, bool hash)
> {
> int error = 0;
> sector_t offset;
> @@ -454,6 +486,7 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
> if (!handle->cur)
> return -EINVAL;
> offset = alloc_swapdev_block(root_swap);
> + crypto_shash_update(handle->desc, buf, PAGE_SIZE);
Was this supposed to be conditionalized behind the new parameter, ie:
if (hash) crypto_shash_update()? If so, the same comment would apply
to the read as well.
