linux-kernel - Re: [PATCH] ACPI: APEI: EINJ: Fix resource leak by remove callback in .exit.text

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: <CAJZ5v0hSoG8tTvY+im+h0ObUhM5_GDWbUa=6Doc1SRRNnuxfiw@mail.gmail.com>
Date: Mon, 18 Aug 2025 18:20:48 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Uwe Kleine-König <u.kleine-koenig@...libre.com>
Cc: Dan Williams <dan.j.williams@...el.com>, 
	Jonathan Cameron <Jonathan.Cameron@...wei.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Ira Weiny <ira.weiny@...el.com>, 
	Zaid Alali <zaidal@...amperecomputing.com>, Colin Ian King <colin.i.king@...il.com>, 
	Dan Carpenter <dan.carpenter@...aro.org>, Sudeep Holla <sudeep.holla@....com>, 
	Tony Luck <tony.luck@...el.com>, Borislav Petkov <bp@...en8.de>, Hanjun Guo <guohanjun@...wei.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Shuai Xue <xueshuai@...ux.alibaba.com>, 
	Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org, 
	sudeep.holla@....covm, Danilo Krummrich <dakr@...nel.org>
Subject: Re: [PATCH] ACPI: APEI: EINJ: Fix resource leak by remove callback in .exit.text

On Thu, Aug 14, 2025 at 7:12 AM Uwe Kleine-König
<u.kleine-koenig@...libre.com> wrote:
>
> The .remove() callback is also used during error handling in
> faux_probe(). As einj_remove() was marked with __exit it's not linked
> into the kernel if the driver is built-in, potentially resulting in
> resource leaks.
>
> Also remove the comment justifying the __exit annotation which doesn't
> apply any more since the driver was converted to the faux device
> interface.
>
> Fixes: 6cb9441bfe8d ("ACPI: APEI: EINJ: Transition to the faux device interface")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...libre.com>

Until the faux device code is updated, this is needed, so applied as
6.17-rc material.

Thanks!

> ---
> Hello,
>
> note that the intention seems to be that this construct is safe, see
> commit c393befa14ab ("driver core: faux: Suppress bind attributes").
> Note further that I don't have a machine to test that so this is only
> based on code reading. An appropriate test might be:
>
> | diff --git a/drivers/base/faux.c b/drivers/base/faux.c
> | index f5fbda0a9a44..decb15f1194a 100644
> | --- a/drivers/base/faux.c
> | +++ b/drivers/base/faux.c
> | @@ -39,6 +39,8 @@ static int faux_match(struct device *dev, const struct device_driver *drv)
> |       return 1;
> |  }
> |
> | +static int once;
> | +
> |  static int faux_probe(struct device *dev)
> |  {
> |       struct faux_object *faux_obj = to_faux_object(dev);
> | @@ -56,7 +58,11 @@ static int faux_probe(struct device *dev)
> |        * Add groups after the probe succeeds to ensure resources are
> |        * initialized correctly
> |        */
> | -     ret = device_add_groups(dev, faux_obj->groups);
> | +
> | +     if (once++)
> | +             ret = -ENOMEM;
> | +     else
> | +             ret = device_add_groups(dev, faux_obj->groups);
> |       if (ret && faux_ops && faux_ops->remove)
> |               faux_ops->remove(faux_dev);
>
> (quoted to make sure that this hunk won't be used when the patch is
> applied).
>
> Even if the faux device interface is fixed not to rely on .remove() the
> comment in einj-core.c needs some love.
>
> Best regards
> Uwe
>
>  drivers/acpi/apei/einj-core.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/apei/einj-core.c b/drivers/acpi/apei/einj-core.c
> index bf8dc92a373a..1204fa3df285 100644
> --- a/drivers/acpi/apei/einj-core.c
> +++ b/drivers/acpi/apei/einj-core.c
> @@ -1091,7 +1091,7 @@ static int __init einj_probe(struct faux_device *fdev)
>         return rc;
>  }
>
> -static void __exit einj_remove(struct faux_device *fdev)
> +static void einj_remove(struct faux_device *fdev)
>  {
>         struct apei_exec_context ctx;
>
> @@ -1114,15 +1114,9 @@ static void __exit einj_remove(struct faux_device *fdev)
>  }
>
>  static struct faux_device *einj_dev;
> -/*
> - * einj_remove() lives in .exit.text. For drivers registered via
> - * platform_driver_probe() this is ok because they cannot get unbound at
> - * runtime. So mark the driver struct with __refdata to prevent modpost
> - * triggering a section mismatch warning.
> - */
> -static struct faux_device_ops einj_device_ops __refdata = {
> +static struct faux_device_ops einj_device_ops = {
>         .probe = einj_probe,
> -       .remove = __exit_p(einj_remove),
> +       .remove = einj_remove,
>  };
>
>  static int __init einj_init(void)
>
> base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> --
> 2.50.1
>
>