linux-kernel - Re: [PATCH] PM: console: Fix memory allocation error handling in pm_vt_switch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251014053608.pwlnexeh7mwjrvsc@lcpd911>
Date: Tue, 14 Oct 2025 11:06:08 +0530
From: Dhruva Gole <d-gole@...com>
To: Malaya Kumar Rout <mrout@...hat.com>
CC: <linux-kernel@...r.kernel.org>, <lyude@...hat.com>,
        <malayarout91@...il.com>, "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown
	<lenb@...nel.org>, Pavel Machek <pavel@...nel.org>,
        <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] PM: console: Fix memory allocation error handling in
 pm_vt_switch_required()

On Oct 14, 2025 at 01:00:27 +0530, Malaya Kumar Rout wrote:
>   The pm_vt_switch_required() function fails silently when memory
>   allocation fails, offering no indication to callers that the operation
>   was unsuccessful. This behavior prevents drivers from handling allocation
>   errors correctly or implementing retry mechanisms. By ensuring that
>   failures are reported back to the caller, drivers can make informed
>   decisions, improve robustness, and avoid unexpected behavior during
>   critical power management operations.
> 
>   Change the function signature to return an integer error code and modify
>   the implementation to return -ENOMEM when kmalloc() fails. Update both
>   the function declaration and the inline stub in include/linux/pm.h to
>   maintain consistency across CONFIG_VT_CONSOLE_SLEEP configurations.
> 
>   The function now returns:
>   - 0 on success (including when updating existing entries)
>   - -ENOMEM when memory allocation fails
> 
>   This change improves error reporting without breaking existing callers,
>   as the current callers in drivers/video/fbdev/core/fbmem.c already
>   ignore the return value, making this a backward-compatible improvement.

Not sure why this commit message has been indented, but it's not
a big deal.

> 
> Reviewed-by: Lyude Paul <lyude@...hat.com>

Btw you can't include a R-by tag in the very first revision of the
patch. This needs to come from Lyude on a public mailing list and only
then can it be picked up.

> Signed-off-by: Malaya Kumar Rout <mrout@...hat.com>
> ---
>  include/linux/pm.h     | 5 +++--
>  kernel/power/console.c | 8 ++++++--
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index cc7b2dc28574..a72e42eec130 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -25,11 +25,12 @@ extern void (*pm_power_off)(void);
>  
>  struct device; /* we have a circular dep with device.h */
>  #ifdef CONFIG_VT_CONSOLE_SLEEP
> -extern void pm_vt_switch_required(struct device *dev, bool required);
> +extern int pm_vt_switch_required(struct device *dev, bool required);
>  extern void pm_vt_switch_unregister(struct device *dev);
>  #else
> -static inline void pm_vt_switch_required(struct device *dev, bool required)
> +static inline int pm_vt_switch_required(struct device *dev, bool required)
>  {
> +	return 0;
>  }
>  static inline void pm_vt_switch_unregister(struct device *dev)
>  {
> diff --git a/kernel/power/console.c b/kernel/power/console.c
> index 19c48aa5355d..a906a0ac0f9b 100644
> --- a/kernel/power/console.c
> +++ b/kernel/power/console.c
> @@ -44,9 +44,10 @@ static LIST_HEAD(pm_vt_switch_list);
>   * no_console_suspend argument has been passed on the command line, VT
>   * switches will occur.
>   */
> -void pm_vt_switch_required(struct device *dev, bool required)
> +int pm_vt_switch_required(struct device *dev, bool required)
>  {
>  	struct pm_vt_switch *entry, *tmp;
> +	int ret = 0;
>  
>  	mutex_lock(&vt_switch_mutex);
>  	list_for_each_entry(tmp, &pm_vt_switch_list, head) {
> @@ -58,8 +59,10 @@ void pm_vt_switch_required(struct device *dev, bool required)
>  	}
>  
>  	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> -	if (!entry)
> +	if (!entry) {
> +		ret = -ENOMEM;
>  		goto out;
> +		}
>  
>  	entry->required = required;
>  	entry->dev = dev;
> @@ -67,6 +70,7 @@ void pm_vt_switch_required(struct device *dev, bool required)
>  	list_add(&entry->head, &pm_vt_switch_list);
>  out:
>  	mutex_unlock(&vt_switch_mutex);
> +	return ret;

I am fine with the overall improved error handling,
Reviewed-by: Dhruva Gole <d-gole@...com>

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated