[PATCH for-4.4.y 0/4] selftests: firmware: fix mainline test runs

[PATCH for-4.4.y 0/4] selftests: firmware: fix mainline test runs

[Amit Pundir]

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(-)

-- 
2.7.4

[PATCH for-4.4.y 1/4] test: firmware_class: report errors properly on failure

[Amit Pundir]

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);
 
-- 
2.7.4

[PATCH for-4.4.y 2/4] selftests: firmware: add empty string and async tests

[Amit Pundir]

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
-- 
2.7.4

[PATCH for-4.4.y 3/4] selftests: firmware: send expected errors to /dev/null

[Amit Pundir]

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
-- 
2.7.4

[PATCH for-4.4.y 4/4] tools: firmware: check for distro fallback udev cancel rule

[Amit Pundir]

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
-- 
2.7.4

Re: [PATCH for-4.4.y 0/4] selftests: firmware: fix mainline test runs

[Greg KH]

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

Re: [PATCH for-4.4.y 0/4] selftests: firmware: fix mainline test runs

[Shuah Khan]

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