Skip to contentSkip to conversation

fix leakes in request_firmware_nowait

PATCH In review matthieu castet 2 participants 4 messages Nov 10, 2005 – Nov 12, 2005 Driver core

lore
Share & tools
lore

matthieu castet

author
#0 lore raw

PATCH 1/1 fix leakes in request_firmware_nowait

Hi,

request_firmware_nowait wasn't checking return error and forgot to free memory in some case.

This patch should fix it.

Signed-off-by Matthieu CASTET <[email protected]>
drivers/base/firmware_class.c +8 -1
Index: linux-2.6.14/drivers/base/firmware_class.c
===================================================================
@@ -511,13 +511,18 @@
 {
 	struct firmware_work *fw_work = arg;
 	const struct firmware *fw;
+	int ret;
 	if (!arg) {
 		WARN_ON(1);
 		return 0;
 	}
 	daemonize("%s/%s", "firmware", fw_work->name);
-	_request_firmware(&fw, fw_work->name, fw_work->device,
+	ret = _request_firmware(&fw, fw_work->name, fw_work->device,
 		fw_work->hotplug);
+	if (ret < 0) {
+		fw_work->cont(NULL, fw_work->context);
+		return ret;
+	}
 	fw_work->cont(fw, fw_work->context);
 	release_firmware(fw);
 	module_put(fw_work->module);

@@ -573,6 +578,8 @@
 
 	if (ret < 0) {
 		fw_work->cont(NULL, fw_work->context);
+		module_put(fw_work->module);
+		kfree(fw_work);
 		return ret;
 	}
 	return 0;
author
#1 lore raw
Quoted 10 lines · matthieu castet

Hi,

request_firmware_nowait wasn't checking return error and forgot to free memory in some case.

This patch should fix it.

Signed-off-by Matthieu CASTET <[email protected]>

And I forgot a case when _request_firmware return an error.

This one should be correct.

Signed-off-by Matthieu CASTET <[email protected]>
drivers/base/firmware_class.c +11 -4
Index: linux-2.6.14/drivers/base/firmware_class.c
===================================================================
@@ -511,18 +511,23 @@
 {
 	struct firmware_work *fw_work = arg;
 	const struct firmware *fw;
+	int ret;
 	if (!arg) {
 		WARN_ON(1);
 		return 0;
 	}
 	daemonize("%s/%s", "firmware", fw_work->name);
-	_request_firmware(&fw, fw_work->name, fw_work->device,
+	ret = _request_firmware(&fw, fw_work->name, fw_work->device,
 		fw_work->hotplug);
-	fw_work->cont(fw, fw_work->context);
-	release_firmware(fw);
+	if (ret < 0)
+		fw_work->cont(NULL, fw_work->context);
+	else {
+		fw_work->cont(fw, fw_work->context);
+		release_firmware(fw);
+	}
 	module_put(fw_work->module);
 	kfree(fw_work);
-	return 0;
+	return ret;
 }
 
 /**

@@ -573,6 +578,8 @@
 
 	if (ret < 0) {
 		fw_work->cont(NULL, fw_work->context);
+		module_put(fw_work->module);
+		kfree(fw_work);
 		return ret;
 	}
 	return 0;
#2 lore raw
Quoted 17 lines · matthieu castet
(quoted diff) +0 -0
@@ -511,18 +511,23 @@
  {
  	struct firmware_work *fw_work = arg;
  	const struct firmware *fw;
 +	int ret;
  	if (!arg) {
  		WARN_ON(1);
  		return 0;
  	}
  	daemonize("%s/%s", "firmware", fw_work->name);
 -	_request_firmware(&fw, fw_work->name, fw_work->device,
 +	ret = _request_firmware(&fw, fw_work->name, fw_work->device,
  		fw_work->hotplug);
 -	fw_work->cont(fw, fw_work->context);
 -	release_firmware(fw);
 +	if (ret < 0)

Review

Quoted 10 lines
(quoted diff) +0 -0
 +		fw_work->cont(NULL, fw_work->context);
 +	else {
 +		fw_work->cont(fw, fw_work->context);
 +		release_firmware(fw);
 +	}
  	module_put(fw_work->module);
  	kfree(fw_work);
 -	return 0;
 +	return ret;
  }

What does the call to fw_work->cont(NULL, ...) do?

author
#3 lore raw
Quoted 6 lines · Andrew Morton

matthieu castet <[email protected]> wrote:

What does the call to fw_work->cont(NULL, ...) do?

It tells to the callback that the firmware request fails [1].

Matthieu

[1]
/**
* request_firmware_nowait:
*
* Description:
* Asynchronous variant of request_firmware() for contexts where
* it is not possible to sleep.
*
* @hotplug invokes hotplug event to copy the firmware image if
this flag
* is non-zero else the firmware copy must be done manually.
*
* @cont will be called asynchronously when the firmware request
is over.
*
* @context will be passed over to @cont.
*
* @fw may be %NULL if firmware request fails.
*
**/
int
request_firmware_nowait(
struct module *module, int hotplug,
const char *name, struct device *device, void *context, void (*cont)(const struct firmware *fw, void *context))