Re: [PATCH 1/1] PM: fix oops in suspend/hibernate code

[Jiri Slaby <jirislaby@gmail.com>]

On 01/06/2011 10:18 AM, Jiri Slaby wrote:
> When ioremap fails (which might happen for some reason), we nicely
> oops in suspend_nvs_save due to NULL dereference by memcpy in there.
> Fail gracefully instead.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> ---

Just to note, I'm not 100% sure whether the fail paths are OK with this
patch. But it works for me.

So it needs an ACK from Rafael or whoever wrote/understand the code.

>  drivers/acpi/sleep.c    |    5 ++---
>  include/linux/suspend.h |    4 ++--
>  kernel/power/nvs.c      |    8 +++++++-
>  3 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index c423231..f94c9a9 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -124,8 +124,7 @@ static int acpi_pm_freeze(void)
>  static int acpi_pm_pre_suspend(void)
>  {
>  	acpi_pm_freeze();
> -	suspend_nvs_save();
> -	return 0;
> +	return suspend_nvs_save();
>  }
>  
>  /**
> @@ -151,7 +150,7 @@ static int acpi_pm_prepare(void)
>  {
>  	int error = __acpi_pm_prepare();
>  	if (!error)
> -		acpi_pm_pre_suspend();
> +		error = acpi_pm_pre_suspend();
>  
>  	return error;
>  }
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index c1f4998..3ac2551 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -262,7 +262,7 @@ static inline bool system_entering_hibernation(void) { return false; }
>  extern int suspend_nvs_register(unsigned long start, unsigned long size);
>  extern int suspend_nvs_alloc(void);
>  extern void suspend_nvs_free(void);
> -extern void suspend_nvs_save(void);
> +extern int suspend_nvs_save(void);
>  extern void suspend_nvs_restore(void);
>  #else /* CONFIG_SUSPEND_NVS */
>  static inline int suspend_nvs_register(unsigned long a, unsigned long b)
> @@ -271,7 +271,7 @@ static inline int suspend_nvs_register(unsigned long a, unsigned long b)
>  }
>  static inline int suspend_nvs_alloc(void) { return 0; }
>  static inline void suspend_nvs_free(void) {}
> -static inline void suspend_nvs_save(void) {}
> +static inline int suspend_nvs_save(void) {}
>  static inline void suspend_nvs_restore(void) {}
>  #endif /* CONFIG_SUSPEND_NVS */
>  
> diff --git a/kernel/power/nvs.c b/kernel/power/nvs.c
> index 1836db6..57c6fab 100644
> --- a/kernel/power/nvs.c
> +++ b/kernel/power/nvs.c
> @@ -105,7 +105,7 @@ int suspend_nvs_alloc(void)
>  /**
>   *	suspend_nvs_save - save NVS memory regions
>   */
> -void suspend_nvs_save(void)
> +int suspend_nvs_save(void)
>  {
>  	struct nvs_page *entry;
>  
> @@ -114,8 +114,14 @@ void suspend_nvs_save(void)
>  	list_for_each_entry(entry, &nvs_list, node)
>  		if (entry->data) {
>  			entry->kaddr = ioremap(entry->phys_start, entry->size);
> +			if (!entry->kaddr) {
> +				suspend_nvs_free();
> +				return -ENOMEM;
> +			}
>  			memcpy(entry->data, entry->kaddr, entry->size);
>  		}
> +
> +	return 0;
>  }
>  
>  /**


-- 
js