On Thu, Aug 14, 2025 at 7:12 AM Uwe Kleine-König
<u.kleine-koenig@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
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
>
>
