Hi Pavel, Thanks for your time to review my patches. 於 日，2013-08-25 於 18:36 +0200，Pavel Machek 提到：
On Thu 2013-08-22 19:01:51, Lee, Chun-Yi wrote:
This patch add the code for generate/verify signature of snapshot, it put the signature to snapshot header. This approach can support both on userspace hibernate and in-kernel hibernate.
v2: - Due to loaded S4 sign key before ExitBootServices, we need forward key from boot kernel to resume target kernel. So this patch add a empty page in snapshot image, then we keep the pfn of this empty page in snapshot header. When system resume from hibernate, we fill new sign key to this empty page space after snapshot image checked pass. This mechanism let boot kernel can forward new sign key to resume target kernel but don't need write new private key to any other storage, e.g. swap.
Cc: Matthew Garrett <mjg59@srcf.ucam.org> Reviewed-by: Jiri Kosina <jkosina@suse.cz> Signed-off-by: Lee, Chun-Yi <jlee@suse.com> --- kernel/power/power.h | 6 + kernel/power/snapshot.c | 280 +++++++++++++++++++++++++++++++++++++++++++++- kernel/power/swap.c | 14 +++ kernel/power/user.c | 9 ++ 4 files changed, 302 insertions(+), 7 deletions(-)
diff --git a/kernel/power/power.h b/kernel/power/power.h index 69a81d8..84e0b06 100644 --- a/kernel/power/power.h +++ b/kernel/power/power.h @@ -3,6 +3,9 @@ #include <linux/utsname.h> #include <linux/freezer.h>
+/* The maximum length of snapshot signature */ +#define SIG_LENG 512 + struct swsusp_info { struct new_utsname uts; u32 version_code; @@ -11,6 +14,8 @@ struct swsusp_info { unsigned long image_pages; unsigned long pages; unsigned long size; + unsigned long skey_data_buf_pfn; + u8 signature[SIG_LENG]; } __attribute__((aligned(PAGE_SIZE)));
SIG_LEN or SIG_LENGTH. Select one.
I will use SIG_LEN at next version, thanks!
+static int copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm) { struct zone *zone; - unsigned long pfn; + unsigned long pfn, dst_pfn; ... + tfm = crypto_alloc_shash(SNAPSHOT_HASH, 0, 0); + if (IS_ERR(tfm)) { + pr_err("IS_ERR(tfm): %ld", PTR_ERR(tfm)); + return PTR_ERR(tfm); + } + + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); + digest_size = crypto_shash_digestsize(tfm); + digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
Are you sure GFP_KERNEL allocation is okay at this phase of hibernation?
Could the hashing be done at later phase, when writing the image to disk?
Thanks for you point out! Yes, call memory allocate here is not a good design due to it causes garbage in snapshot that will not released by resumed kernel. I just finished another implementation, the respin patch extracts the signature generation code to another function then call the function in swsusp_save() after copy_data_pages() finished. We can write to memory at that stage.
+void **h_buf;
helpfully named.
I will change the name to handle_buffers;
+ ret = verify_signature(s4_wake_key, pks); + if (ret) { + pr_err("snapshot S4 signature verification fail: %d\n", ret); + goto error_verify; + } else + pr_info("snapshot S4 signature verification pass!\n"); + + if (pks->rsa.s) + mpi_free(pks->rsa.s); + kfree(pks);
ret = 0 and fall through?
When verification success, verify_signature() will return 0. Yes, here have duplicate code, I will clear up it.
+ return 0; + +error_verify: + if (pks->rsa.s) + mpi_free(pks->rsa.s); +error_mpi: + kfree(pks); + return ret; +}
+ ret = crypto_shash_final(desc, digest); + if (ret) + goto error_shash; + + ret = snapshot_verify_signature(digest, digest_size); + if (ret) + goto error_verify; + + kfree(h_buf); + kfree(digest); + crypto_free_shash(tfm); + return 0;
These four lines can be deleted.
Yes, here also duplicate, I will remove.
+ +error_verify: +error_shash: + kfree(h_buf); + kfree(digest); +error_digest: + crypto_free_shash(tfm); + return ret; +} + Pavel
Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org