linux-kernel - Re: [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on faux_device

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250609111758.0000258d@huawei.com>
Date: Mon, 9 Jun 2025 11:17:58 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Dan Williams <dan.j.williams@...el.com>
CC: <gregkh@...uxfoundation.org>, <rafael.j.wysocki@...el.com>,
	<linux-cxl@...r.kernel.org>, <linux-acpi@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
	Sudeep Holla <sudeep.holla@....com>, Ben Cheatham <Benjamin.Cheatham@....com>
Subject: Re: [PATCH 3/3] ACPI: APEI: EINJ: Do not fail einj_init() on
 faux_device_create() failure

On Fri, 6 Jun 2025 20:32:28 -0700
Dan Williams <dan.j.williams@...el.com> wrote:

> CXL has a symbol dependency on einj_core.ko, so if einj_init() fails then
> cxl_core.ko fails to load. Prior to the faux_device_create() conversion,
> einj_probe() failures were tracked by the einj_initialized flag without
> failing einj_init().
> 
> Revert to that behavior and always succeed einj_init() given there is no
> way, and no pressing need, to discern faux device-create vs device-probe
> failures.
> 
> This situation arose because CXL knows proper kernel named objects to
> trigger errors against, but acpi-einj knows how to perform the error
> injection. The injection mechanism is shared with non-CXL use cases. The
> result is CXL now has a module dependency on einj-core.ko, and init/probe
> failures are handled at runtime.
> 
> Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@...nel.org>
> Cc: Sudeep Holla <sudeep.holla@....com>
> Cc: Ben Cheatham <Benjamin.Cheatham@....com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>
> ---
>  drivers/acpi/apei/einj-core.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index fea11a35eea3..9b041415a9d0 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -883,19 +883,16 @@ static int __init einj_init(void)
>  	}
>  
>  	einj_dev = faux_device_create("acpi-einj", NULL, &einj_device_ops);
> -	if (!einj_dev)
> -		return -ENODEV;
>  
> -	einj_initialized = true;
> +	if (einj_dev)
> +		einj_initialized = true;
>  
>  	return 0;
>  }
>  
>  static void __exit einj_exit(void)
>  {
> -	if (einj_initialized)
> -		faux_device_destroy(einj_dev);
> -
> +	faux_device_destroy(einj_dev);

Hi Dan,

Thi bit is sort of fine though not really related, because
faux_device_destroy() checks

void faux_device_destroy(struct faux_device *faux_dev)
{
	struct device *dev = &faux_dev->dev;

	if (!faux_dev)
		return;

Though that check is after a dereference of faux_dev
which doesn't look right to me.  Might be fine because
of how the kernel is built (I can't remember where we ended
up on topic of compilers making undefined behavior based
optimizations).  Still not that nice from a logical point of view!

>  }
>  
>  module_init(einj_init);