Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1287960 > unrolled thread
| Started by | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| First post | 2015-12-10 00:00 +0100 |
| Last post | 2015-12-10 00:30 +0100 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 3/5] test: firmware_class: add asynchronous request trigger Brian Norris <computersforpeace@gmail.com> - 2015-12-10 00:00 +0100
Re: [PATCH v2 3/5] test: firmware_class: add asynchronous request trigger Kees Cook <keescook@chromium.org> - 2015-12-10 00:30 +0100
| From | Brian Norris <computersforpeace@gmail.com> |
|---|---|
| Date | 2015-12-10 00:00 +0100 |
| Subject | [PATCH v2 3/5] test: firmware_class: add asynchronous request trigger |
| Message-ID | <qDVSi-65j-9@gated-at.bofh.it> |
We might want to test for bugs like that found in commit f9692b2699bd
("firmware: fix possible use after free on name on asynchronous
request"), where the asynchronous request API had race conditions.
Let's add a simple file that will launch the async request, then wait
until it's complete and report the status. It's not a true async test
(we're using a mutex + wait_for_completion(), so we can't get more than
one going at the same time), but it does help make sure the basic API is
sane, and it can catch some class of bugs.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Cc: Luis R. Rodriguez <mcgrof@suse.com>
---
v2:
* use kstrndup()
* don't kfree(name) too early!
lib/test_firmware.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index 690b9c35a274..a3e8ec3fb1c5 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -12,6 +12,7 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/printk.h>
+#include <linux/completion.h>
#include <linux/firmware.h>
#include <linux/device.h>
#include <linux/fs.h>
@@ -80,6 +81,57 @@ out:
}
static DEVICE_ATTR_WO(trigger_request);
+static DECLARE_COMPLETION(async_fw_done);
+
+static void trigger_async_request_cb(const struct firmware *fw, void *context)
+{
+ test_firmware = fw;
+ complete(&async_fw_done);
+}
+
+static ssize_t trigger_async_request_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int rc;
+ char *name;
+
+ name = kstrndup(buf, count, GFP_KERNEL);
+ if (!name)
+ return -ENOSPC;
+
+ pr_info("loading '%s'\n", name);
+
+ mutex_lock(&test_fw_mutex);
+ release_firmware(test_firmware);
+ test_firmware = NULL;
+ rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL,
+ NULL, trigger_async_request_cb);
+ if (rc) {
+ pr_info("async load of '%s' failed: %d\n", name, rc);
+ kfree(name);
+ goto out;
+ }
+ /* Free 'name' ASAP, to test for race conditions */
+ kfree(name);
+
+ wait_for_completion(&async_fw_done);
+
+ if (test_firmware) {
+ pr_info("loaded: %zu\n", test_firmware->size);
+ rc = count;
+ } else {
+ pr_err("failed to async load firmware\n");
+ rc = -ENODEV;
+ }
+
+out:
+ mutex_unlock(&test_fw_mutex);
+
+ return rc;
+}
+static DEVICE_ATTR_WO(trigger_async_request);
+
static int __init test_firmware_init(void)
{
int rc;
@@ -96,9 +148,20 @@ static int __init test_firmware_init(void)
goto dereg;
}
+ rc = device_create_file(test_fw_misc_device.this_device,
+ &dev_attr_trigger_async_request);
+ if (rc) {
+ pr_err("could not create async sysfs interface: %d\n", rc);
+ goto remove_file;
+ }
+
pr_warn("interface ready\n");
return 0;
+
+remove_file:
+ device_remove_file(test_fw_misc_device.this_device,
+ &dev_attr_trigger_async_request);
dereg:
misc_deregister(&test_fw_misc_device);
return rc;
@@ -110,6 +173,8 @@ static void __exit test_firmware_exit(void)
{
release_firmware(test_firmware);
device_remove_file(test_fw_misc_device.this_device,
+ &dev_attr_trigger_async_request);
+ device_remove_file(test_fw_misc_device.this_device,
&dev_attr_trigger_request);
misc_deregister(&test_fw_misc_device);
pr_warn("removed interface\n");
--
2.6.0.rc2.230.g3dd15c0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2015-12-10 00:30 +0100 |
| Message-ID | <qDWlk-6vk-1@gated-at.bofh.it> |
| In reply to | #1287960 |
On Wed, Dec 9, 2015 at 2:50 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> We might want to test for bugs like that found in commit f9692b2699bd
> ("firmware: fix possible use after free on name on asynchronous
> request"), where the asynchronous request API had race conditions.
>
> Let's add a simple file that will launch the async request, then wait
> until it's complete and report the status. It's not a true async test
> (we're using a mutex + wait_for_completion(), so we can't get more than
> one going at the same time), but it does help make sure the basic API is
> sane, and it can catch some class of bugs.
>
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> Cc: Luis R. Rodriguez <mcgrof@suse.com>
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> v2:
> * use kstrndup()
> * don't kfree(name) too early!
>
> lib/test_firmware.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
>
> diff --git a/lib/test_firmware.c b/lib/test_firmware.c
> index 690b9c35a274..a3e8ec3fb1c5 100644
> --- a/lib/test_firmware.c
> +++ b/lib/test_firmware.c
> @@ -12,6 +12,7 @@
> #include <linux/init.h>
> #include <linux/module.h>
> #include <linux/printk.h>
> +#include <linux/completion.h>
> #include <linux/firmware.h>
> #include <linux/device.h>
> #include <linux/fs.h>
> @@ -80,6 +81,57 @@ out:
> }
> static DEVICE_ATTR_WO(trigger_request);
>
> +static DECLARE_COMPLETION(async_fw_done);
> +
> +static void trigger_async_request_cb(const struct firmware *fw, void *context)
> +{
> + test_firmware = fw;
> + complete(&async_fw_done);
> +}
> +
> +static ssize_t trigger_async_request_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int rc;
> + char *name;
> +
> + name = kstrndup(buf, count, GFP_KERNEL);
> + if (!name)
> + return -ENOSPC;
> +
> + pr_info("loading '%s'\n", name);
> +
> + mutex_lock(&test_fw_mutex);
> + release_firmware(test_firmware);
> + test_firmware = NULL;
> + rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL,
> + NULL, trigger_async_request_cb);
> + if (rc) {
> + pr_info("async load of '%s' failed: %d\n", name, rc);
> + kfree(name);
> + goto out;
> + }
> + /* Free 'name' ASAP, to test for race conditions */
> + kfree(name);
> +
> + wait_for_completion(&async_fw_done);
> +
> + if (test_firmware) {
> + pr_info("loaded: %zu\n", test_firmware->size);
> + rc = count;
> + } else {
> + pr_err("failed to async load firmware\n");
> + rc = -ENODEV;
> + }
> +
> +out:
> + mutex_unlock(&test_fw_mutex);
> +
> + return rc;
> +}
> +static DEVICE_ATTR_WO(trigger_async_request);
> +
> static int __init test_firmware_init(void)
> {
> int rc;
> @@ -96,9 +148,20 @@ static int __init test_firmware_init(void)
> goto dereg;
> }
>
> + rc = device_create_file(test_fw_misc_device.this_device,
> + &dev_attr_trigger_async_request);
> + if (rc) {
> + pr_err("could not create async sysfs interface: %d\n", rc);
> + goto remove_file;
> + }
> +
> pr_warn("interface ready\n");
>
> return 0;
> +
> +remove_file:
> + device_remove_file(test_fw_misc_device.this_device,
> + &dev_attr_trigger_async_request);
> dereg:
> misc_deregister(&test_fw_misc_device);
> return rc;
> @@ -110,6 +173,8 @@ static void __exit test_firmware_exit(void)
> {
> release_firmware(test_firmware);
> device_remove_file(test_fw_misc_device.this_device,
> + &dev_attr_trigger_async_request);
> + device_remove_file(test_fw_misc_device.this_device,
> &dev_attr_trigger_request);
> misc_deregister(&test_fw_misc_device);
> pr_warn("removed interface\n");
> --
> 2.6.0.rc2.230.g3dd15c0
>
--
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web