Hi,
On Fri, Aug 26, 2016 at 09:56:54PM +0200, Pavel Machek wrote:

Hi!

quoted

quoted

quoted

quoted

What's the progress of this patch? Looks already have experts review it.
Why this patch didn't accept?

This patch is a little overkilled, and I have saved another simpler
version to only check the md5 hash (as people suggested) for it. I can post it later.

I am happy to test and review it.

Here it is. As Rafael is on travel, it would be grateful
if you can give some advance on this, thanks!

Better than last one.

quoted

+		return -ENOMEM;
+
+	req = ahash_request_alloc(tfm, GFP_ATOMIC);

what context is this called from? GFP_ATOMIC allocations like to fail...

It is in normal process context, OK, I'll change it to GFP_KERNEL.

quoted

+static int hibernation_e820_check(void *buf)
+{
+	int ret;
+	char result[MD5_HASH_SIZE] = {0};
+
+	ret = get_e820_md5(&e820_saved, result);
+	if (ret)
+		return ret;
+
+	if (memcmp(result, buf, MD5_HASH_SIZE))
+		e820_conflict = true;

Passing return value using global variable is ugly. Can you just print
the warning and kill the box here?

Do you mean get rid of the panic hooker and just print the warning here?

quoted

+

quoted

+	/*
+	 * A page has been allocated previously to store the hibernation
+	 * image header, so we can safely store the md5 result behind
+	 * struct restore_data_record, with size of 128 bytes.
+	 */
+	hibernation_e820_save(addr + sizeof(struct restore_data_record));
+

Please just allocate space in struct restore_data_record . And I don't
think md5 sum is 128 _bytes_.

OK. The md5 sum should be 128 bits thus 16 bytes.

Thanks!
Yu