linux-kernel - [PATCH] firmware: fix async/manual firmware loading

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20161030145048.6291-1-corsac@corsac.net>
Date:   Sun, 30 Oct 2016 15:50:47 +0100
From:   Yves-Alexis Perez <corsac@...sac.net>
To:     linux-kernel@...r.kernel.org
Subject: [PATCH] firmware: fix async/manual firmware loading

Hi,

when trying to (ab)use the firmware loading interface in a local kernel module
(in order to load the EEPROM content into a PCIE card), I noticed that the
manual firmware loading interface (the one from
/sys/class/firmware/<foo>/{loading,data}) was broken.

After instrumenting the kernel I noticed an issue with the way
wait_for_completion_interruptible_timeout() is called in _request_firmware()
and especially how the return value is handled: it's supposed to be a long, but
here it's silently casted to an int and tested if positive. The initial timeout
seems to be LONG_MAX (since it's a manual firmware loading you're supposed to
have all the time you want to do it), so the return value overflows the int.

Attached patch fixes the problem here, although there might be a better way. I
tested it using lib/test_firmware.c kernel module, with the following patch to
enable manual loading:

diff --git a/lib/test_firmware.c b/lib/test_firmware.c
index a3e8ec3..01d333c 100644
--- a/lib/test_firmware.c
+++ b/lib/test_firmware.c
@@ -105,7 +105,7 @@ static ssize_t trigger_async_request_store(struct device *dev,
        mutex_lock(&test_fw_mutex);
        release_firmware(test_firmware);
        test_firmware = NULL;
-       rc = request_firmware_nowait(THIS_MODULE, 1, name, dev, GFP_KERNEL,
+       rc = request_firmware_nowait(THIS_MODULE, 0, name, dev, GFP_KERNEL,
                                     NULL, trigger_async_request_cb);
        if (rc) {
                pr_info("async load of '%s' failed: %d\n", name, rc);

Then load test_firmware and:

# echo -n test > /sys/devices/virtual/misc/test_firmware/trigger_async_request

In another terminal, do:

# echo -n 1 > /sys/class/firmware/test/loading
# echo -n data > /sys/class/firmware/test/data
# echo -n 0 > /sys/class/firmware/test/loading

Without the patch, the loading fails right after the "echo -n 0", with it the loading succeeds with:

[   96.405171] test_firmware: loaded: 4

Regards,
-- 
Yves-Alexis