Re: [PATCH] ACPI / APEI: Add missing synchronize_rcu() on NOTIFY_SCI removal.

["Huang\, Ying" <ying.huang@intel.com>]

Hi, James,

James Morse <james.morse@arm.com> writes:

> When removing a GHES device notified by SCI, list_del_rcu() is used,
> ghes_remove() should call synchronize_rcu() before it goes on to call
> kfree(ghes), otherwise concurrent RCU readers may still hold this list
> entry after it has been freed.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Cc: Huang Ying <ying.huang@intel.com>
>
> ---
> It looks like 81e88fdc432a lifted this into ACPI_HEST_NOTIFY_NMI, missing
> that ACPI_HEST_NOTIFY_SCI needed it too.
>
> If there is only ever one SCI GHES entry this is safe today as
> unregister_acpi_hed_notifier() takes a write lock on its semaphore, meaning
> any RCU readers will have finished.
> If there can be more than one SCI GHES entry...
>
> Fixes: 81e88fdc432a ("ACPI, APEI, Generic Hardware Error Source POLL/IRQ/NMI notification type support")
>
>  drivers/acpi/apei/ghes.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index b192b42a8351..79b3c9c5a3bc 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1073,6 +1073,7 @@ static int ghes_remove(struct platform_device *ghes_dev)
>  		if (list_empty(&ghes_sci))
>  			unregister_acpi_hed_notifier(&ghes_notifier_sci);

In remove path

unregister_acpi_hed_notifier()
  blocking_notifier_chain_unregister()
    down_write(&nh->rwsem)

While in notifier call path

acpi_hed_notify()
  blocking_notifier_call_chain()
    __blocking_notifier_call_chain()
      down_read(&nh->rwsem)

So when unregister succeeds, the notifier call should have
finished.

Best Regards,
Huang, Ying

>  		mutex_unlock(&ghes_list_mutex);
> +		synchronize_rcu();
>  		break;
>  	case ACPI_HEST_NOTIFY_NMI:
>  		ghes_nmi_remove(ghes);