Hi Greg,
Please consider following upstream selftest firmware changes to help run mainline/linux-next firmware test scripts on 4.4.y, without breaking 0-day testing.
47e0bbb7fa98 ("test: firmware_class: report errors properly on failure") is the one patch actually needed to make sure mainline/linux-next selftest firmware test scripts run on 4.4.y.
Following commits are, however, required to make sure the above commit doesn't break 0-day testing, wherein they run in-kernel selftests. 1b1fe542b6f0 ("selftests: firmware: add empty string and async tests") 880444e214cf ("selftests: firmware: send expected errors to /dev/null") afb999cdef69 ("tools: firmware: check for distro fallback udev cancel rule")
Regards, Amit Pundir
Brian Norris (2): test: firmware_class: report errors properly on failure selftests: firmware: add empty string and async tests
Luis R. Rodriguez (2): selftests: firmware: send expected errors to /dev/null tools: firmware: check for distro fallback udev cancel rule
lib/test_firmware.c | 11 ++++++--- tools/testing/selftests/firmware/fw_filesystem.sh | 10 +++++++- tools/testing/selftests/firmware/fw_userhelper.sh | 28 +++++++++++++++++++++-- 3 files changed, 43 insertions(+), 6 deletions(-)
From: Brian Norris computersforpeace@gmail.com
commit 47e0bbb7fa985a0f1b5794a8653fae4f8f49de77 upstream.
request_firmware() failures currently won't get reported at all (the error code is discarded). What's more, we get confusing messages, like:
# echo -n notafile > /sys/devices/virtual/misc/test_firmware/trigger_request [ 8280.311856] test_firmware: loading 'notafile' [ 8280.317042] test_firmware: load of 'notafile' failed: -2 [ 8280.322445] test_firmware: loaded: 0 # echo $? 0
Report the failures via write() errors, and don't say we "loaded" anything.
Signed-off-by: Brian Norris computersforpeace@gmail.com Acked-by: Kees Cook keescook@chromium.org Signed-off-by: Shuah Khan shuahkh@osg.samsung.com Signed-off-by: Amit Pundir amit.pundir@linaro.org --- lib/test_firmware.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 86374c1c49a4..841191061816 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -65,14 +65,19 @@ static ssize_t trigger_request_store(struct device *dev, release_firmware(test_firmware); test_firmware = NULL; rc = request_firmware(&test_firmware, name, dev); - if (rc) + if (rc) { pr_info("load of '%s' failed: %d\n", name, rc); - pr_info("loaded: %zu\n", test_firmware ? test_firmware->size : 0); + goto out; + } + pr_info("loaded: %zu\n", test_firmware->size); + rc = count; + +out: mutex_unlock(&test_fw_mutex);
kfree(name);
- return count; + return rc; } static DEVICE_ATTR_WO(trigger_request);
From: Brian Norris computersforpeace@gmail.com
commit 1b1fe542b6f010cf6bc7e1c92805e1c0e133e007 upstream.
Now that we've added a 'trigger_async_request' knob to test the request_firmware_nowait() API, let's use it. Also add tests for the empty ("") string, since there have been a couple errors in that handling already.
Since we now have real ways that the sysfs write might fail, let's add the appropriate check on the 'echo' lines too.
Signed-off-by: Brian Norris computersforpeace@gmail.com Acked-by: Kees Cook keescook@chromium.org Signed-off-by: Shuah Khan shuahkh@osg.samsung.com [AmitP: Dropped the async trigger testing parts from original commit] Signed-off-by: Amit Pundir amit.pundir@linaro.org --- tools/testing/selftests/firmware/fw_filesystem.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index c4366dc74e01..e9f563fd200a 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -48,8 +48,16 @@ echo "ABCD0123" >"$FW"
NAME=$(basename "$FW")
+if printf '\000' >"$DIR"/trigger_request; then + echo "$0: empty filename should not succeed" >&2 + exit 1 +fi + # Request a firmware that doesn't exist, it should fail. -echo -n "nope-$NAME" >"$DIR"/trigger_request +if echo -n "nope-$NAME" >"$DIR"/trigger_request; then + echo "$0: firmware shouldn't have loaded" >&2 + exit 1 +fi if diff -q "$FW" /dev/test_firmware >/dev/null ; then echo "$0: firmware was not expected to match" >&2 exit 1
From: "Luis R. Rodriguez" mcgrof@kernel.org
commit 880444e214cfd293a2e8cc4bd3505f7ffa6ce33a upstream.
Error that we expect should not be spilled to stdout.
Without this we get:
./fw_filesystem.sh: line 58: printf: write error: Invalid argument ./fw_filesystem.sh: line 63: printf: write error: No such device ./fw_filesystem.sh: line 69: echo: write error: No such file or directory ./fw_filesystem.sh: filesystem loading works ./fw_filesystem.sh: async filesystem loading works
With it:
./fw_filesystem.sh: filesystem loading works ./fw_filesystem.sh: async filesystem loading works
Signed-off-by: Luis R. Rodriguez mcgrof@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [AmitP: Dropped the async trigger testing parts from original commit] Signed-off-by: Amit Pundir amit.pundir@linaro.org --- tools/testing/selftests/firmware/fw_filesystem.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index e9f563fd200a..856a1f327b3f 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -48,13 +48,13 @@ echo "ABCD0123" >"$FW"
NAME=$(basename "$FW")
-if printf '\000' >"$DIR"/trigger_request; then +if printf '\000' >"$DIR"/trigger_request 2> /dev/null; then echo "$0: empty filename should not succeed" >&2 exit 1 fi
# Request a firmware that doesn't exist, it should fail. -if echo -n "nope-$NAME" >"$DIR"/trigger_request; then +if echo -n "nope-$NAME" >"$DIR"/trigger_request 2> /dev/null; then echo "$0: firmware shouldn't have loaded" >&2 exit 1 fi
From: "Luis R. Rodriguez" mcgrof@kernel.org
commit afb999cdef69148f366839e74470d8f5375ba5f1 upstream.
Some distributions (Debian, OpenSUSE) have a udev rule in place to cancel all fallback mechanism uevents immediately. This would obviously make it hard to test against the fallback mechanism test interface, so we need to check for this.
Signed-off-by: Luis R. Rodriguez mcgrof@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Amit Pundir amit.pundir@linaro.org --- tools/testing/selftests/firmware/fw_userhelper.sh | 28 +++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/firmware/fw_userhelper.sh b/tools/testing/selftests/firmware/fw_userhelper.sh index b9983f8e09f6..01c626a1f226 100755 --- a/tools/testing/selftests/firmware/fw_userhelper.sh +++ b/tools/testing/selftests/firmware/fw_userhelper.sh @@ -64,9 +64,33 @@ trap "test_finish" EXIT echo "ABCD0123" >"$FW" NAME=$(basename "$FW")
+DEVPATH="$DIR"/"nope-$NAME"/loading + # Test failure when doing nothing (timeout works). -echo 1 >/sys/class/firmware/timeout -echo -n "$NAME" >"$DIR"/trigger_request +echo -n 2 >/sys/class/firmware/timeout +echo -n "nope-$NAME" >"$DIR"/trigger_request 2>/dev/null & + +# Give the kernel some time to load the loading file, must be less +# than the timeout above. +sleep 1 +if [ ! -f $DEVPATH ]; then + echo "$0: fallback mechanism immediately cancelled" + echo "" + echo "The file never appeared: $DEVPATH" + echo "" + echo "This might be a distribution udev rule setup by your distribution" + echo "to immediately cancel all fallback requests, this must be" + echo "removed before running these tests. To confirm look for" + echo "a firmware rule like /lib/udev/rules.d/50-firmware.rules" + echo "and see if you have something like this:" + echo "" + echo "SUBSYSTEM=="firmware", ACTION=="add", ATTR{loading}="-1"" + echo "" + echo "If you do remove this file or comment out this line before" + echo "proceeding with these tests." + exit 1 +fi + if diff -q "$FW" /dev/test_firmware >/dev/null ; then echo "$0: firmware was not expected to match" >&2 exit 1
On Thu, Nov 09, 2017 at 01:24:08AM +0530, Amit Pundir wrote:
Hi Greg,
Please consider following upstream selftest firmware changes to help run mainline/linux-next firmware test scripts on 4.4.y, without breaking 0-day testing.
47e0bbb7fa98 ("test: firmware_class: report errors properly on failure") is the one patch actually needed to make sure mainline/linux-next selftest firmware test scripts run on 4.4.y.
Following commits are, however, required to make sure the above commit doesn't break 0-day testing, wherein they run in-kernel selftests. 1b1fe542b6f0 ("selftests: firmware: add empty string and async tests") 880444e214cf ("selftests: firmware: send expected errors to /dev/null") afb999cdef69 ("tools: firmware: check for distro fallback udev cancel rule")
Now all applied, and I added the last two to 4.9 so as to not get out of sync.
thanks,
greg k-h
On 11/10/2017 07:05 AM, Greg KH wrote:
On Thu, Nov 09, 2017 at 01:24:08AM +0530, Amit Pundir wrote:
Hi Greg,
Please consider following upstream selftest firmware changes to help run mainline/linux-next firmware test scripts on 4.4.y, without breaking 0-day testing.
47e0bbb7fa98 ("test: firmware_class: report errors properly on failure") is the one patch actually needed to make sure mainline/linux-next selftest firmware test scripts run on 4.4.y.
Following commits are, however, required to make sure the above commit doesn't break 0-day testing, wherein they run in-kernel selftests. 1b1fe542b6f0 ("selftests: firmware: add empty string and async tests") 880444e214cf ("selftests: firmware: send expected errors to /dev/null") afb999cdef69 ("tools: firmware: check for distro fallback udev cancel rule")
Now all applied, and I added the last two to 4.9 so as to not get out of sync.
thanks,
greg k-h
Thanks both for taking care of this.
-- Shuah
linux-stable-mirror@lists.linaro.org
-
Amit Pundir -
Greg KH -
Shuah Khan