[PATCH] firmware_loader: avoid UAF on buggy request_firmware_nowait() users

[Luis Chamberlain]

request_firmware_nowait() is documented as requiring the caller to
ensure to maintain the the reference count of @device during the
lifetime of the call to request_firmware_nowait() and the callback.

It would seem drivers exist which don't follow these rules though,
and things like syzbot can trigger UAF if the device gets nuked
as request_firmware_nowait() is being called. Instead of enabling
use UAF, defend against such improperly written drivers and complain
about it.

Make the documentaiton a bit clearer and give a hint as to how to easily
accomplish device lifetime maintenance on the driver using a completion
and a wait_for_completion().

Fixes: 0cfc1e1e7b534 ("firmware loader: fix device lifetime")
Fixes: f8a4bd3456b98 ("firmware loader: embed device into firmware_priv structure")
Cc: stable@xxxxxxxxxxxxxxx # v2.6.36
Reported-by: syzbot+6bc35f3913193fe7f0d3@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
---
 drivers/base/firmware_loader/main.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 7c3590fd97c2..6ac92dfdd85e 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -1118,15 +1118,16 @@ static void request_firmware_work_func(struct work_struct *work)
  * @uevent: sends uevent to copy the firmware image if this flag
  *	is non-zero else the firmware copy must be done manually.
  * @name: name of firmware file
- * @device: device for which firmware is being loaded
+ * @device: device for which firmware is being loaded. The caller must hold
+ * 	the reference count of @device during the lifetime of this routine
+ * 	and the @cont callback. This typically can be done with a completion
+ * 	and wait_for_completion prior to device teardown.
  * @gfp: allocation flags
  * @context: will be passed over to @cont, and
  *	@fw may be %NULL if firmware request fails.
  * @cont: function will be called asynchronously when the firmware
  *	request is over.
  *
- *	Caller must hold the reference count of @device.
- *
  *	Asynchronous variant of request_firmware() for user contexts:
  *		- sleep for as small periods as possible since it may
  *		  increase kernel boot time of built-in device drivers
@@ -1171,7 +1172,12 @@ request_firmware_nowait(
 		return -EFAULT;
 	}
 
-	get_device(fw_work->device);
+	if (WARN_ON(!get_device(fw_work->device))) {
+		module_put(module);
+		kfree_const(fw_work->name);
+		kfree(fw_work);
+		return -ENODEV;
+	}
 	INIT_WORK(&fw_work->work, request_firmware_work_func);
 	schedule_work(&fw_work->work);
 	return 0;
-- 
2.35.1