Re: [PATCH v2 1/2] soc: xilinx: Fix race condition in event registration

[Michal Simek <michal.simek@amd.com>]

On 3/20/26 07:03, Prasanna Kumar T S M wrote:
> The zynqmp_power driver registers handlers for suspend and subsystem
> restart events using register_event(). However, the work structures
> (zynqmp_pm_init_suspend_work and zynqmp_pm_init_restart_work) used by
> these handlers were allocated and initialized after the registration
> call.
> 
> This created a race window where, if the firmware triggered an event
> immediately after registration but before allocation, the callback
> (suspend_event_callback or subsystem_restart_event_callback) would
> dereference a NULL pointer in work_pending(), leading to a crash.
> 
> Fix this by allocating and initializing the work structures before
> registering the events.
> 
> Fixes: fcf544ac6439 ("soc: xilinx: Add cb event for subsystem restart")
> Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> ---
>   drivers/soc/xilinx/zynqmp_power.c | 43 ++++++++++++-------------------
>   1 file changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/soc/xilinx/zynqmp_power.c b/drivers/soc/xilinx/zynqmp_power.c
> index 9085db1b480a..9dd938bd01d8 100644
> --- a/drivers/soc/xilinx/zynqmp_power.c
> +++ b/drivers/soc/xilinx/zynqmp_power.c
> @@ -303,18 +303,18 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>   	 * is not available to use) or -ENODEV(Xilinx Event Manager not compiled),
>   	 * then use ipi-mailbox or interrupt method.
>   	 */
> +	zynqmp_pm_init_suspend_work = devm_kzalloc(&pdev->dev,
> +						   sizeof(struct zynqmp_pm_work_struct),
> +						   GFP_KERNEL);
> +	if (!zynqmp_pm_init_suspend_work)
> +		return -ENOMEM;
> +
> +	INIT_WORK(&zynqmp_pm_init_suspend_work->callback_work,
> +		  zynqmp_pm_init_suspend_work_fn);
> +
>   	ret = register_event(&pdev->dev, PM_INIT_SUSPEND_CB, 0, 0, false,
>   			     suspend_event_callback);
>   	if (!ret) {
> -		zynqmp_pm_init_suspend_work = devm_kzalloc(&pdev->dev,
> -							   sizeof(struct zynqmp_pm_work_struct),
> -							   GFP_KERNEL);
> -		if (!zynqmp_pm_init_suspend_work)
> -			return -ENOMEM;
> -
> -		INIT_WORK(&zynqmp_pm_init_suspend_work->callback_work,
> -			  zynqmp_pm_init_suspend_work_fn);
> -
>   		ret = zynqmp_pm_get_family_info(&pm_family_code);
>   		if (ret < 0)
>   			return ret;
> @@ -326,14 +326,6 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>   		else
>   			return -ENODEV;
>   
> -		ret = register_event(&pdev->dev, PM_NOTIFY_CB, node_id, EVENT_SUBSYSTEM_RESTART,
> -				     false, subsystem_restart_event_callback);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n",
> -				ret);
> -			return ret;
> -		}
> -
>   		zynqmp_pm_init_restart_work = devm_kzalloc(&pdev->dev,
>   							   sizeof(struct zynqmp_pm_work_struct),
>   							   GFP_KERNEL);
> @@ -342,19 +334,18 @@ static int zynqmp_pm_probe(struct platform_device *pdev)
>   
>   		INIT_WORK(&zynqmp_pm_init_restart_work->callback_work,
>   			  zynqmp_pm_subsystem_restart_work_fn);
> +
> +		ret = register_event(&pdev->dev, PM_NOTIFY_CB, node_id, EVENT_SUBSYSTEM_RESTART,
> +				     false, subsystem_restart_event_callback);
> +		if (ret) {
> +			dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n",
> +				ret);
> +			return ret;
> +		}
>   	} else if (ret != -EACCES && ret != -ENODEV) {
>   		dev_err(&pdev->dev, "Failed to Register with Xilinx Event manager %d\n", ret);
>   		return ret;
>   	} else if (of_property_present(pdev->dev.of_node, "mboxes")) {
> -		zynqmp_pm_init_suspend_work =
> -			devm_kzalloc(&pdev->dev,
> -				     sizeof(struct zynqmp_pm_work_struct),
> -				     GFP_KERNEL);
> -		if (!zynqmp_pm_init_suspend_work)
> -			return -ENOMEM;
> -
> -		INIT_WORK(&zynqmp_pm_init_suspend_work->callback_work,
> -			  zynqmp_pm_init_suspend_work_fn);
>   		client = devm_kzalloc(&pdev->dev, sizeof(*client), GFP_KERNEL);
>   		if (!client)
>   			return -ENOMEM;


Applied both.

Thanks,
Michal