From d3edeb47d886cf38bcf0e96c38f15f6cfda67e02 Mon Sep 17 00:00:00 2001 From: Bamvor Jian Zhang Date: Tue, 17 Nov 2015 22:35:41 +0800 Subject: [PATCH 01/14] selftests/capabilities: clean up for Makefile Clean up the following things: 1. Avoid the broken when use TARGETS in the command line, eg: $ make -C tools/testing/selftests TARGETS=capabilities make[1]: *** No rule to make target 'capabilities', needed by 'all'. Stop. Replace TARGETS with BINARIES. 2. User need to provide cap-ng.h and libcap-ng.so for cross compiling. Replace ':=' with '+=' for CFLAGS and introduce LDLIBS to archieve it. Delete useless EXTRA_CLAGS at the same time. 3. Delete the duplicated definition which is already defined by lib.mk. Suggested-by: Michael Ellerman Signed-off-by: Bamvor Jian Zhang Signed-off-by: Shuah Khan --- tools/testing/selftests/capabilities/Makefile | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/tools/testing/selftests/capabilities/Makefile b/tools/testing/selftests/capabilities/Makefile index 8c8f0c1f0889..008602aed920 100644 --- a/tools/testing/selftests/capabilities/Makefile +++ b/tools/testing/selftests/capabilities/Makefile @@ -1,18 +1,15 @@ -all: +TEST_FILES := validate_cap +TEST_PROGS := test_execve + +BINARIES := $(TEST_FILES) $(TEST_PROGS) + +CFLAGS += -O2 -g -std=gnu99 -Wall +LDLIBS += -lcap-ng -lrt -ldl + +all: $(BINARIES) + +clean: + $(RM) $(BINARIES) include ../lib.mk -.PHONY: all clean - -TARGETS := validate_cap test_execve -TEST_PROGS := test_execve - -CFLAGS := -O2 -g -std=gnu99 -Wall -lcap-ng - -all: $(TARGETS) - -clean: - $(RM) $(TARGETS) - -$(TARGETS): %: %.c - $(CC) -o $@ $(CFLAGS) $(EXTRA_CFLAGS) $^ -lrt -ldl From f4ecb322ab890a5c907a129fcee47056bf523415 Mon Sep 17 00:00:00 2001 From: Bamvor Jian Zhang Date: Tue, 17 Nov 2015 22:35:42 +0800 Subject: [PATCH 02/14] selftests/capabilities: actually test it The capatabilities exist in selftest but no in the TARGETS list. Add it to the TARGETS. Signed-off-by: Bamvor Jian Zhang Signed-off-by: Shuah Khan --- tools/testing/selftests/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index c8edff6803d1..ec0ecadbae99 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -1,4 +1,5 @@ TARGETS = breakpoints +TARGETS += capabilities TARGETS += cpu-hotplug TARGETS += efivarfs TARGETS += exec From 7e722473811a4f82a48bb2ef934ff44ca9ab8fa5 Mon Sep 17 00:00:00 2001 From: Bamvor Jian Zhang Date: Tue, 17 Nov 2015 22:35:43 +0800 Subject: [PATCH 03/14] selftest/ipc: actually test it The ipc testcase exist in selftest but no in the TARGETS list. Add it to the TARGETS. Signed-off-by: Bamvor Jian Zhang Signed-off-by: Shuah Khan --- tools/testing/selftests/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index ec0ecadbae99..b04afc3295df 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -6,6 +6,7 @@ TARGETS += exec TARGETS += firmware TARGETS += ftrace TARGETS += futex +TARGETS += ipc TARGETS += kcmp TARGETS += lib TARGETS += membarrier From ed2d26d7cbccb1aa719a5d3fb8e87ef6610cb530 Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Wed, 18 Nov 2015 12:04:48 -0500 Subject: [PATCH 04/14] tools, testing, add test for intel_pstate driver This test used the cpupower utility to set the cpu frequency from the maximum turbo value to the minimum supported value in steps of 100 MHz. The results are displayed in a table which indicate the "Target" state, or the requested frequency in MHz, the Actual frequency, as read from /proc/cpuinfo, the difference between the Target and Actual frequencies, and the value of MSR 0x199 (MSR_IA32_PERF_CTL) which indicates what pstate the cpu is in, and the value of /sys/devices/system/cpu/intel_pstate/max_perf_pct X maximum turbo state Cc: Shuah Khan Cc: linux-api@vger.kernel.org Signed-off-by: Prarit Bhargava Signed-off-by: Shuah Khan --- tools/testing/selftests/intel_pstate/Makefile | 15 +++ tools/testing/selftests/intel_pstate/aperf.c | 80 +++++++++++++ tools/testing/selftests/intel_pstate/msr.c | 39 ++++++ tools/testing/selftests/intel_pstate/run.sh | 113 ++++++++++++++++++ 4 files changed, 247 insertions(+) create mode 100644 tools/testing/selftests/intel_pstate/Makefile create mode 100644 tools/testing/selftests/intel_pstate/aperf.c create mode 100644 tools/testing/selftests/intel_pstate/msr.c create mode 100755 tools/testing/selftests/intel_pstate/run.sh diff --git a/tools/testing/selftests/intel_pstate/Makefile b/tools/testing/selftests/intel_pstate/Makefile new file mode 100644 index 000000000000..f5f1a28715ff --- /dev/null +++ b/tools/testing/selftests/intel_pstate/Makefile @@ -0,0 +1,15 @@ +CC := $(CROSS_COMPILE)gcc +CFLAGS := $(CFLAGS) -Wall -D_GNU_SOURCE +LDFLAGS := $(LDFLAGS) -lm + +TARGETS := msr aperf + +TEST_PROGS := $(TARGETS) run.sh + +.PHONY: all clean +all: $(TARGETS) + +$(TARGETS): $(HEADERS) + +clean: + rm -f $(TARGETS) diff --git a/tools/testing/selftests/intel_pstate/aperf.c b/tools/testing/selftests/intel_pstate/aperf.c new file mode 100644 index 000000000000..6046e183f4ad --- /dev/null +++ b/tools/testing/selftests/intel_pstate/aperf.c @@ -0,0 +1,80 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +void usage(char *name) { + printf ("Usage: %s cpunum\n", name); +} + +int main(int argc, char **argv) { + int i, cpu, fd; + char msr_file_name[64]; + long long tsc, old_tsc, new_tsc; + long long aperf, old_aperf, new_aperf; + long long mperf, old_mperf, new_mperf; + struct timeb before, after; + long long int start, finish, total; + cpu_set_t cpuset; + + if (argc != 2) { + usage(argv[0]); + return 1; + } + + errno = 0; + cpu = strtol(argv[1], (char **) NULL, 10); + + if (errno) { + usage(argv[0]); + return 1; + } + + sprintf(msr_file_name, "/dev/cpu/%d/msr", cpu); + fd = open(msr_file_name, O_RDONLY); + + if (fd == -1) { + perror("Failed to open"); + return 1; + } + + CPU_ZERO(&cpuset); + CPU_SET(cpu, &cpuset); + + if (sched_setaffinity(0, sizeof(cpu_set_t), &cpuset)) { + perror("Failed to set cpu affinity"); + return 1; + } + + ftime(&before); + pread(fd, &old_tsc, sizeof(old_tsc), 0x10); + pread(fd, &old_aperf, sizeof(old_mperf), 0xe7); + pread(fd, &old_mperf, sizeof(old_aperf), 0xe8); + + for (i=0; i<0x8fffffff; i++) { + sqrt(i); + } + + ftime(&after); + pread(fd, &new_tsc, sizeof(new_tsc), 0x10); + pread(fd, &new_aperf, sizeof(new_mperf), 0xe7); + pread(fd, &new_mperf, sizeof(new_aperf), 0xe8); + + tsc = new_tsc-old_tsc; + aperf = new_aperf-old_aperf; + mperf = new_mperf-old_mperf; + + start = before.time*1000 + before.millitm; + finish = after.time*1000 + after.millitm; + total = finish - start; + + printf("runTime: %4.2f\n", 1.0*total/1000); + printf("freq: %7.0f\n", tsc / (1.0*aperf / (1.0 * mperf)) / total); + return 0; +} diff --git a/tools/testing/selftests/intel_pstate/msr.c b/tools/testing/selftests/intel_pstate/msr.c new file mode 100644 index 000000000000..abbbfc84d359 --- /dev/null +++ b/tools/testing/selftests/intel_pstate/msr.c @@ -0,0 +1,39 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +int main(int argc, char **argv) { + int cpu, fd; + long long msr; + char msr_file_name[64]; + + if (argc != 2) + return 1; + + errno = 0; + cpu = strtol(argv[1], (char **) NULL, 10); + + if (errno) + return 1; + + sprintf(msr_file_name, "/dev/cpu/%d/msr", cpu); + fd = open(msr_file_name, O_RDONLY); + + if (fd == -1) { + perror("Failed to open"); + return 1; + } + + pread(fd, &msr, sizeof(msr), 0x199); + + printf("msr 0x199: 0x%llx\n", msr); + return 0; +} diff --git a/tools/testing/selftests/intel_pstate/run.sh b/tools/testing/selftests/intel_pstate/run.sh new file mode 100755 index 000000000000..bdaf37e92684 --- /dev/null +++ b/tools/testing/selftests/intel_pstate/run.sh @@ -0,0 +1,113 @@ +#!/bin/bash +# +# This test runs on Intel x86 based hardware which support the intel_pstate +# driver. The test checks the frequency settings from the maximum turbo +# state to the minimum supported frequency, in decrements of 100MHz. The +# test runs the aperf.c program to put load on each processor. +# +# The results are displayed in a table which indicate the "Target" state, +# or the requested frequency in MHz, the Actual frequency, as read from +# /proc/cpuinfo, the difference between the Target and Actual frequencies, +# and the value of MSR 0x199 (MSR_IA32_PERF_CTL) which indicates what +# pstate the cpu is in, and the value of +# /sys/devices/system/cpu/intel_pstate/max_perf_pct X maximum turbo state +# +# Notes: In some cases several frequency values may be placed in the +# /tmp/result.X files. This is done on purpose in order to catch cases +# where the pstate driver may not be working at all. There is the case +# where, for example, several "similar" frequencies are in the file: +# +# +#/tmp/result.3100:1:cpu MHz : 2899.980 +#/tmp/result.3100:2:cpu MHz : 2900.000 +#/tmp/result.3100:3:msr 0x199: 0x1e00 +#/tmp/result.3100:4:max_perf_pct 94 +# +# and the test will error out in those cases. The result.X file can be checked +# for consistency and modified to remove the extra MHz values. The result.X +# files can be re-evaluated by setting EVALUATE_ONLY to 1 below. + +EVALUATE_ONLY=0 + +max_cpus=$(($(nproc)-1)) + +# compile programs +gcc -o aperf aperf.c -lm +[ $? -ne 0 ] && echo "Problem compiling aperf.c." && exit 1 +gcc -o msr msr.c -lm +[ $? -ne 0 ] && echo "Problem compiling msr.c." && exit 1 + +function run_test () { + + file_ext=$1 + for cpu in `seq 0 $max_cpus` + do + echo "launching aperf load on $cpu" + ./aperf $cpu & + done + + echo "sleeping for 5 seconds" + sleep 5 + num_freqs=$(cat /proc/cpuinfo | grep MHz | sort -u | wc -l) + if [ $num_freqs -le 2 ]; then + cat /proc/cpuinfo | grep MHz | sort -u | tail -1 > /tmp/result.$1 + else + cat /proc/cpuinfo | grep MHz | sort -u > /tmp/result.$1 + fi + ./msr 0 >> /tmp/result.$1 + + max_perf_pct=$(cat /sys/devices/system/cpu/intel_pstate/max_perf_pct) + echo "max_perf_pct $max_perf_pct" >> /tmp/result.$1 + + for job in `jobs -p` + do + echo "waiting for job id $job" + wait $job + done +} + +# +# MAIN (ALL UNITS IN MHZ) +# + +# Get the marketing frequency +_mkt_freq=$(cat /proc/cpuinfo | grep -m 1 "model name" | awk '{print $NF}') +_mkt_freq=$(echo $_mkt_freq | tr -d [:alpha:][:punct:]) +mkt_freq=${_mkt_freq}0 + +# Get the ranges from cpupower +_min_freq=$(cpupower frequency-info -l | tail -1 | awk ' { print $1 } ') +min_freq=$(($_min_freq / 1000)) +_max_freq=$(cpupower frequency-info -l | tail -1 | awk ' { print $2 } ') +max_freq=$(($_max_freq / 1000)) + + +for freq in `seq $max_freq -100 $min_freq` +do + echo "Setting maximum frequency to $freq" + cpupower frequency-set -g powersave --max=${freq}MHz >& /dev/null + [ $EVALUATE_ONLY -eq 0 ] && run_test $freq +done + +echo "==============================================================================" + +echo "The marketing frequency of the cpu is $mkt_freq MHz" +echo "The maximum frequency of the cpu is $max_freq MHz" +echo "The minimum frequency of the cpu is $min_freq MHz" + +cpupower frequency-set -g powersave --max=${max_freq}MHz >& /dev/null + +# make a pretty table +echo "Target Actual Difference MSR(0x199) max_perf_pct" +for freq in `seq $max_freq -100 $min_freq` +do + result_freq=$(cat /tmp/result.${freq} | grep "cpu MHz" | awk ' { print $4 } ' | awk -F "." ' { print $1 } ') + msr=$(cat /tmp/result.${freq} | grep "msr" | awk ' { print $3 } ') + max_perf_pct=$(cat /tmp/result.${freq} | grep "max_perf_pct" | awk ' { print $2 } ' ) + if [ $result_freq -eq $freq ]; then + echo " $freq $result_freq 0 $msr $(($max_perf_pct*3300))" + else + echo " $freq $result_freq $(($result_freq-$freq)) $msr $(($max_perf_pct*$max_freq))" + fi +done +exit 0 From 27f87a8775d264eaeb9b0c7ec11ae4d7a9d8b2ac Mon Sep 17 00:00:00 2001 From: Yuan Sun Date: Thu, 7 Jan 2016 14:53:59 +0800 Subject: [PATCH 05/14] update .gitignore in selftests/vm Signed-off-by: Yuan Sun Signed-off-by: Shuah Khan --- tools/testing/selftests/vm/.gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/testing/selftests/vm/.gitignore b/tools/testing/selftests/vm/.gitignore index ff1bb16cec4f..a937a9d26b60 100644 --- a/tools/testing/selftests/vm/.gitignore +++ b/tools/testing/selftests/vm/.gitignore @@ -2,3 +2,8 @@ hugepage-mmap hugepage-shm map_hugetlb thuge-gen +compaction_test +mlock2-tests +on-fault-limit +transhuge-stress +userfaultfd From 886187c4a12b5a75c146f8477d96e6b308cd0fe6 Mon Sep 17 00:00:00 2001 From: Yuan Sun Date: Thu, 7 Jan 2016 14:54:00 +0800 Subject: [PATCH 06/14] update .gitignore in selftests/timers Signed-off-by: Yuan Sun Signed-off-by: Shuah Khan --- tools/testing/selftests/timers/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/timers/.gitignore b/tools/testing/selftests/timers/.gitignore index ced998151bc4..68f3fc71ac44 100644 --- a/tools/testing/selftests/timers/.gitignore +++ b/tools/testing/selftests/timers/.gitignore @@ -16,3 +16,4 @@ set-timer-lat skew_consistency threadtest valid-adjtimex +adjtick From f4cbbcc28476cc6cbb3903fd794d62cc0710bd77 Mon Sep 17 00:00:00 2001 From: Yuan Sun Date: Thu, 7 Jan 2016 14:54:01 +0800 Subject: [PATCH 07/14] add ptrace/.gitignore Signed-off-by: Yuan Sun Signed-off-by: Shuah Khan --- tools/testing/selftests/ptrace/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 tools/testing/selftests/ptrace/.gitignore diff --git a/tools/testing/selftests/ptrace/.gitignore b/tools/testing/selftests/ptrace/.gitignore new file mode 100644 index 000000000000..b3e59d41fd82 --- /dev/null +++ b/tools/testing/selftests/ptrace/.gitignore @@ -0,0 +1 @@ +peeksiginfo From cdf88b4f45142f7c85a934f05b55a6c08e1d2414 Mon Sep 17 00:00:00 2001 From: Yuan Sun Date: Thu, 7 Jan 2016 14:54:02 +0800 Subject: [PATCH 08/14] add breakpoints/.gitignore Signed-off-by: Yuan Sun Signed-off-by: Shuah Khan --- tools/testing/selftests/breakpoints/.gitignore | 1 + 1 file changed, 1 insertion(+) create mode 100644 tools/testing/selftests/breakpoints/.gitignore diff --git a/tools/testing/selftests/breakpoints/.gitignore b/tools/testing/selftests/breakpoints/.gitignore new file mode 100644 index 000000000000..9b3193d06608 --- /dev/null +++ b/tools/testing/selftests/breakpoints/.gitignore @@ -0,0 +1 @@ +breakpoint_test From b5bb6d3068eabb075ee7db09c73374f6db73ff4a Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 10 Dec 2015 14:50:25 -0800 Subject: [PATCH 09/14] selftests/seccomp: fix 32-bit build warnings The casting was done incorrectly for 32-bit builds. Fixed to use uintptr_t. Reported-by: Eric Adams Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/seccomp/test_harness.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/seccomp/test_harness.h b/tools/testing/selftests/seccomp/test_harness.h index fb2841601f2f..a786c69c7584 100644 --- a/tools/testing/selftests/seccomp/test_harness.h +++ b/tools/testing/selftests/seccomp/test_harness.h @@ -42,6 +42,7 @@ #define TEST_HARNESS_H_ #define _GNU_SOURCE +#include #include #include #include @@ -370,8 +371,8 @@ __typeof__(_expected) __exp = (_expected); \ __typeof__(_seen) __seen = (_seen); \ if (!(__exp _t __seen)) { \ - unsigned long long __exp_print = (unsigned long long)__exp; \ - unsigned long long __seen_print = (unsigned long long)__seen; \ + unsigned long long __exp_print = (uintptr_t)__exp; \ + unsigned long long __seen_print = (uintptr_t)__seen; \ __TH_LOG("Expected %s (%llu) %s %s (%llu)", \ #_expected, __exp_print, #_t, \ #_seen, __seen_print); \ From 47e0bbb7fa985a0f1b5794a8653fae4f8f49de77 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Wed, 9 Dec 2015 14:50:25 -0800 Subject: [PATCH 10/14] test: firmware_class: report errors properly on failure 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 Acked-by: Kees Cook Signed-off-by: Shuah Khan --- 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 be4a1326d12cce8df1f57017bf4112eaab437a38 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Wed, 9 Dec 2015 14:50:26 -0800 Subject: [PATCH 11/14] test: firmware_class: use kstrndup() where appropriate We're essentially just doing an open-coded kstrndup(). The only differences are with what happens after the first '\0' character, but request_firmware() doesn't care about that. Suggested-by: Kees Cook Signed-off-by: Brian Norris Signed-off-by: Shuah Khan --- lib/test_firmware.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/test_firmware.c b/lib/test_firmware.c index 841191061816..690b9c35a274 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -54,10 +54,9 @@ static ssize_t trigger_request_store(struct device *dev, int rc; char *name; - name = kzalloc(count + 1, GFP_KERNEL); + name = kstrndup(buf, count, GFP_KERNEL); if (!name) return -ENOSPC; - memcpy(name, buf, count); pr_info("loading '%s'\n", name); From eb910947c82f93cee6ee00a59a4ed86f12cf6e7f Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Wed, 9 Dec 2015 14:50:27 -0800 Subject: [PATCH 12/14] test: firmware_class: add asynchronous request trigger 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 Cc: Luis R. Rodriguez Acked-by: Kees Cook Signed-off-by: Shuah Khan --- 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 #include #include +#include #include #include #include @@ -80,6 +81,57 @@ static ssize_t trigger_request_store(struct device *dev, } 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; @@ -109,6 +172,8 @@ module_init(test_firmware_init); 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); From 715780ae4bb76d6fd2f20eb78e2a9ba9769a6cdc Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Wed, 9 Dec 2015 14:50:28 -0800 Subject: [PATCH 13/14] firmware: actually return NULL on failed request_firmware_nowait() The kerneldoc for request_firmware_nowait() says that it may call the provided cont() callback with @fw == NULL, if the firmware request fails. However, this is not the case when called with an empty string (""). This case is short-circuited by the 'name[0] == '\0'' check introduced in commit 471b095dfe0d ("firmware_class: make sure fw requests contain a name"), so _request_firmware() never gets to set the fw to NULL. Noticed while using the new 'trigger_async_request' testing hook: # printf '\x00' > /sys/devices/virtual/misc/test_firmware/trigger_async_request [10553.726178] test_firmware: loading '' [10553.729859] test_firmware: loaded: 995209091 # printf '\x00' > /sys/devices/virtual/misc/test_firmware/trigger_async_request [10733.676184] test_firmware: loading '' [10733.679855] Unable to handle kernel NULL pointer dereference at virtual address 00000004 [10733.687951] pgd = ec188000 [10733.690655] [00000004] *pgd=00000000 [10733.694240] Internal error: Oops: 5 [#1] SMP ARM [10733.698847] Modules linked in: btmrvl_sdio btmrvl bluetooth sbs_battery nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables asix usbnet mwifiex_sdio mwifiex cfg80211 jitterentropy_rng drbg joydev snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device ppp_async ppp_generic slhc tun [10733.725670] CPU: 0 PID: 6600 Comm: bash Not tainted 4.4.0-rc4-00351-g63d0877 #178 [10733.733137] Hardware name: Rockchip (Device Tree) [10733.737831] task: ed24f6c0 ti: ee322000 task.ti: ee322000 [10733.743222] PC is at do_raw_spin_lock+0x18/0x1a0 [10733.747831] LR is at _raw_spin_lock+0x18/0x1c [10733.752180] pc : [] lr : [] psr: a00d0013 [10733.752180] sp : ee323df8 ip : ee323e20 fp : ee323e1c [10733.763634] r10: 00000051 r9 : b6f18000 r8 : ee323f80 [10733.768847] r7 : c089cebc r6 : 00000001 r5 : 00000000 r4 : ec0e6000 [10733.775360] r3 : dead4ead r2 : c06bd140 r1 : eef913b4 r0 : 00000000 [10733.781874] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [10733.788995] Control: 10c5387d Table: 2c18806a DAC: 00000051 [10733.794728] Process bash (pid: 6600, stack limit = 0xee322218) [10733.800549] Stack: (0xee323df8 to 0xee324000) [10733.804896] 3de0: ec0e6000 00000000 [10733.813059] 3e00: 00000001 c089cebc ee323f80 b6f18000 ee323e2c ee323e20 c054c204 c0065394 [10733.821221] 3e20: ee323e44 ee323e30 c02fec60 c054c1f8 ec0e7ec0 ec3fcfc0 ee323e5c ee323e48 [10733.829384] 3e40: c02fed08 c02fec48 c07dbf74 eeb05a00 ee323e8c ee323e60 c0253828 c02fecac [10733.837547] 3e60: 00000001 c0116950 ee323eac ee323e78 00000001 ec3fce00 ed2d9700 ed2d970c [10733.845710] 3e80: ee323e9c ee323e90 c02e873c c02537d4 ee323eac ee323ea0 c017bd40 c02e8720 [10733.853873] 3ea0: ee323ee4 ee323eb0 c017b250 c017bd00 00000000 00000000 f3e47a54 ec128b00 [10733.862035] 3ec0: c017b10c ee323f80 00000001 c000f504 ee322000 00000000 ee323f4c ee323ee8 [10733.870197] 3ee0: c011b71c c017b118 ee323fb0 c011bc90 becfa8d9 00000001 ec128b00 00000001 [10733.878359] 3f00: b6f18000 ee323f80 ee323f4c ee323f18 c011bc90 c0063950 ee323f3c ee323f28 [10733.886522] 3f20: c0063950 c0549138 00000001 ec128b00 00000001 ec128b00 b6f18000 ee323f80 [10733.894684] 3f40: ee323f7c ee323f50 c011bed8 c011b6ec c0135fb8 c0135f24 ec128b00 ec128b00 [10733.902847] 3f60: 00000001 b6f18000 c000f504 ee322000 ee323fa4 ee323f80 c011c664 c011be24 [10733.911009] 3f80: 00000000 00000000 00000001 b6f18000 b6e79be0 00000004 00000000 ee323fa8 [10733.919172] 3fa0: c000f340 c011c618 00000001 b6f18000 00000001 b6f18000 00000001 00000000 [10733.927334] 3fc0: 00000001 b6f18000 b6e79be0 00000004 00000001 00000001 8068a3f1 b6e79c84 [10733.935496] 3fe0: 00000000 becfa7dc b6de194d b6e20246 400d0030 00000001 7a4536e8 49bda390 [10733.943664] [] (do_raw_spin_lock) from [] (_raw_spin_lock+0x18/0x1c) [10733.951743] [] (_raw_spin_lock) from [] (fw_free_buf+0x24/0x64) [10733.959388] [] (fw_free_buf) from [] (release_firmware+0x68/0x74) [10733.967207] [] (release_firmware) from [] (trigger_async_request_store+0x60/0x124) [10733.976501] [] (trigger_async_request_store) from [] (dev_attr_store+0x28/0x34) [10733.985533] [] (dev_attr_store) from [] (sysfs_kf_write+0x4c/0x58) [10733.993437] [] (sysfs_kf_write) from [] (kernfs_fop_write+0x144/0x1a8) [10734.001689] [] (kernfs_fop_write) from [] (__vfs_write+0x3c/0xe4) After this patch: # printf '\x00' > /sys/devices/virtual/misc/test_firmware/trigger_async_request [ 32.126322] test_firmware: loading '' [ 32.129995] test_firmware: failed to async load firmware -bash: printf: write error: No such device Fixes: 471b095dfe0d ("firmware_class: make sure fw requests contain a name") Signed-off-by: Brian Norris Acked-by: Ming Lei Acked-by: Kees Cook Signed-off-by: Shuah Khan --- drivers/base/firmware_class.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 8524450e75bd..b9250e564ebf 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -1118,15 +1118,17 @@ static int _request_firmware(const struct firmware **firmware_p, const char *name, struct device *device, unsigned int opt_flags) { - struct firmware *fw; + struct firmware *fw = NULL; long timeout; int ret; if (!firmware_p) return -EINVAL; - if (!name || name[0] == '\0') - return -EINVAL; + if (!name || name[0] == '\0') { + ret = -EINVAL; + goto out; + } ret = _request_firmware_prepare(&fw, name, device); if (ret <= 0) /* error or already assigned */ From 1b1fe542b6f010cf6bc7e1c92805e1c0e133e007 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Wed, 9 Dec 2015 14:50:29 -0800 Subject: [PATCH 14/14] selftests: firmware: add empty string and async tests 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 Acked-by: Kees Cook Signed-off-by: Shuah Khan --- .../selftests/firmware/fw_filesystem.sh | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index c4366dc74e01..5c495ad7958a 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -48,8 +48,21 @@ echo "ABCD0123" >"$FW" NAME=$(basename "$FW") +if printf '\000' >"$DIR"/trigger_request; then + echo "$0: empty filename should not succeed" >&2 + exit 1 +fi + +if printf '\000' >"$DIR"/trigger_async_request; then + echo "$0: empty filename should not succeed (async)" >&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 @@ -74,4 +87,18 @@ else echo "$0: filesystem loading works" fi +# Try the asynchronous version too +if ! echo -n "$NAME" >"$DIR"/trigger_async_request ; then + echo "$0: could not trigger async request" >&2 + exit 1 +fi + +# Verify the contents are what we expect. +if ! diff -q "$FW" /dev/test_firmware >/dev/null ; then + echo "$0: firmware was not loaded (async)" >&2 + exit 1 +else + echo "$0: async filesystem loading works" +fi + exit 0