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

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