linux-kernel - Re: [PATCH] mmc: pwrseq: Use proper reboot notifier path

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <676cf135-ab3e-48d8-9fdf-83276502b58a@samsung.com>
Date: Wed, 31 Jan 2024 09:27:47 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Andrew Davis <afd@...com>, Ulf Hansson <ulf.hansson@...aro.org>, Yangtao
	Li <frank.li@...o.com>
Cc: linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mmc: pwrseq: Use proper reboot notifier path

On 26.01.2024 20:01, Andrew Davis wrote:
> This driver registers itself as a reboot handler, which means it claims
> it can reboot the system. It does this so it is called during the system
> reboot sequence. The correct way to be notified during the reboot
> sequence is to register a notifier with register_reboot_notifier().
> Do this here.
>
> Note this will be called during normal reboots but not emergency reboots.
> This is the expected behavior, emergency reboot means emergency, not go
> do some cleanup with emmc pins.. The reboot notifiers are intentionally
> not called in the emergency path for a reason and working around that by
> pretending to be a reboot handler is a hack.


Well, I'm the author of this 'hack' and unfortunately there was no other 
way to make emergency reboot working on boards requiring the eMMC 
pwrseq. IIRC this has been already discussed and the conclusion was to 
accept the hack with the comments explaining the problem.


> Signed-off-by: Andrew Davis <afd@...com>
> ---
>   drivers/mmc/core/pwrseq_emmc.c | 8 +-------
>   1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
> index 3b6d69cefb4eb..d5045fd1a02c1 100644
> --- a/drivers/mmc/core/pwrseq_emmc.c
> +++ b/drivers/mmc/core/pwrseq_emmc.c
> @@ -70,14 +70,8 @@ static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
>   		return PTR_ERR(pwrseq->reset_gpio);
>   
>   	if (!gpiod_cansleep(pwrseq->reset_gpio)) {
> -		/*
> -		 * register reset handler to ensure emmc reset also from
> -		 * emergency_reboot(), priority 255 is the highest priority
> -		 * so it will be executed before any system reboot handler.
> -		 */
>   		pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
> -		pwrseq->reset_nb.priority = 255;
> -		register_restart_handler(&pwrseq->reset_nb);
> +		register_reboot_notifier(&pwrseq->reset_nb);
>   	} else {
>   		dev_notice(dev, "EMMC reset pin tied to a sleepy GPIO driver; reset on emergency-reboot disabled\n");
>   	}

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland