2020-07-20 07:22:05

by Li Heng

[permalink] [raw]
Subject: [PATCH] efi: add missed destroy_workqueue when efisubsys_init fails

destroy_workqueue() should be called to destroy efi_rts_wq
when efisubsys_init() init resources fails.

Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Li Heng <[email protected]>
---
drivers/firmware/efi/efi.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 5114cae..f8cce41 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -379,6 +379,7 @@ static int __init efisubsys_init(void)
efi_kobj = kobject_create_and_add("efi", firmware_kobj);
if (!efi_kobj) {
pr_err("efi: Firmware registration failed.\n");
+ destroy_workqueue(efi_rts_wq);
return -ENOMEM;
}

@@ -420,6 +421,7 @@ static int __init efisubsys_init(void)
generic_ops_unregister();
err_put:
kobject_put(efi_kobj);
+ destroy_workqueue(efi_rts_wq);
return error;
}

--
2.7.4


2020-07-23 06:29:59

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH] efi: add missed destroy_workqueue when efisubsys_init fails

> destroy_workqueue() should be called to destroy efi_rts_wq
> when efisubsys_init() init resources fails.

* Can such exception handling depend on the data structure member “efi.runtime_supported_mask”?

* An imperative wording would be preferred for the change description
(besides another bit of fine-tuning), wouldn't it?

* Will the tag “Fixes” become helpful for the commit message?


…
> +++ b/drivers/firmware/efi/efi.c
> @@ -379,6 +379,7 @@ static int __init efisubsys_init(void)
> efi_kobj = kobject_create_and_add("efi", firmware_kobj);
> if (!efi_kobj) {
> pr_err("efi: Firmware registration failed.\n");
> + destroy_workqueue(efi_rts_wq);
> return -ENOMEM;
> }

How do you think about to use the following statements instead
in the if branch?

- return -ENOMEM;
+ error = -ENOMEM;
+ goto destroy_workqueue;


Regards,
Markus

Subject: [tip: efi/urgent] efi: add missed destroy_workqueue when efisubsys_init fails

The following commit has been merged into the efi/urgent branch of tip:

Commit-ID: 98086df8b70c06234a8f4290c46064e44dafa0ed
Gitweb: https://git.kernel.org/tip/98086df8b70c06234a8f4290c46064e44dafa0ed
Author: Li Heng <[email protected]>
AuthorDate: Mon, 20 Jul 2020 15:22:18 +08:00
Committer: Ard Biesheuvel <[email protected]>
CommitterDate: Thu, 20 Aug 2020 11:18:45 +02:00

efi: add missed destroy_workqueue when efisubsys_init fails

destroy_workqueue() should be called to destroy efi_rts_wq
when efisubsys_init() init resources fails.

Cc: <[email protected]>
Reported-by: Hulk Robot <[email protected]>
Signed-off-by: Li Heng <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/efi.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index fdd1db0..3aa07c3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -381,6 +381,7 @@ static int __init efisubsys_init(void)
efi_kobj = kobject_create_and_add("efi", firmware_kobj);
if (!efi_kobj) {
pr_err("efi: Firmware registration failed.\n");
+ destroy_workqueue(efi_rts_wq);
return -ENOMEM;
}

@@ -424,6 +425,7 @@ err_unregister:
generic_ops_unregister();
err_put:
kobject_put(efi_kobj);
+ destroy_workqueue(efi_rts_wq);
return error;
}