| Message ID | afb1c18493fa3fb891a37752161e9f73a46fc2df.1536685746.git.yu.c.chen@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Fixes hibernation bugs on x86-32 system | expand |
On Wed, 12 Sep 2018, Chen Yu wrote: > static bool hibernation_e820_mismatch(void *buf) > @@ -306,6 +307,7 @@ static bool hibernation_e820_mismatch(void *buf) > int arch_hibernation_header_save(void *addr, unsigned int max_size) > { > struct restore_data_record *rdr = addr; > + int ret = -EINVAL; What's the point of initializing ret? > if (max_size < sizeof(struct restore_data_record)) > return -EOVERFLOW; > @@ -333,7 +335,9 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size) > > rdr->magic = RESTORE_MAGIC; > > - hibernation_e820_save(rdr->e820_digest); > + ret = hibernation_e820_save(rdr->e820_digest); > + if (ret) > + return ret; > > return 0; And what;s the point of ret at all? return hibernation_e820_save(); is effectivly the same. Thanks, tglx
Hi Thomas, On Thu, Sep 13, 2018 at 10:26:39AM +0200, Thomas Gleixner wrote: > On Wed, 12 Sep 2018, Chen Yu wrote: > > static bool hibernation_e820_mismatch(void *buf) > > @@ -306,6 +307,7 @@ static bool hibernation_e820_mismatch(void *buf) > > int arch_hibernation_header_save(void *addr, unsigned int max_size) > > { > > struct restore_data_record *rdr = addr; > > + int ret = -EINVAL; > > What's the point of initializing ret? > > > if (max_size < sizeof(struct restore_data_record)) > > return -EOVERFLOW; > > @@ -333,7 +335,9 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size) > > > > rdr->magic = RESTORE_MAGIC; > > > > - hibernation_e820_save(rdr->e820_digest); > > + ret = hibernation_e820_save(rdr->e820_digest); > > + if (ret) > > + return ret; > > > > return 0; > > And what;s the point of ret at all? > > return hibernation_e820_save(); > > is effectivly the same. > ret is useless, will change to 'return hibernation_e820_save()' here. Thanks, Yu > Thanks, > > tglx
diff --git a/arch/x86/power/hibernate_64.c b/arch/x86/power/hibernate_64.c index f8e3b668d20b..dc12dc57889e 100644 --- a/arch/x86/power/hibernate_64.c +++ b/arch/x86/power/hibernate_64.c @@ -265,9 +265,9 @@ static int get_e820_md5(struct e820_table *table, void *buf) return ret; } -static void hibernation_e820_save(void *buf) +static int hibernation_e820_save(void *buf) { - get_e820_md5(e820_table_firmware, buf); + return get_e820_md5(e820_table_firmware, buf); } static bool hibernation_e820_mismatch(void *buf) @@ -287,8 +287,9 @@ static bool hibernation_e820_mismatch(void *buf) return memcmp(result, buf, MD5_DIGEST_SIZE) ? true : false; } #else -static void hibernation_e820_save(void *buf) +static int hibernation_e820_save(void *buf) { + return 0; } static bool hibernation_e820_mismatch(void *buf) @@ -306,6 +307,7 @@ static bool hibernation_e820_mismatch(void *buf) int arch_hibernation_header_save(void *addr, unsigned int max_size) { struct restore_data_record *rdr = addr; + int ret = -EINVAL; if (max_size < sizeof(struct restore_data_record)) return -EOVERFLOW; @@ -333,7 +335,9 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size) rdr->magic = RESTORE_MAGIC; - hibernation_e820_save(rdr->e820_digest); + ret = hibernation_e820_save(rdr->e820_digest); + if (ret) + return ret; return 0; }
Currently if get_e820_md5() fails, then it will hibernate nevertheless. Actually the error code should be propagated to upper caller so that the hibernation could be aware of the result and terminates the process if md5 digest fails. Suggested-by: Thomas Gleixner <tglx@linutronix.de> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Signed-off-by: Chen Yu <yu.c.chen@intel.com> --- arch/x86/power/hibernate_64.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)