This moves the usermode helper locks into only code paths that use the
usermode helper API from the kernel. The usermode helper locks were
originally added to prevent stalling suspend, later the firmware cache
was added to help with this, and further later direct filesystem lookup
was added by Linus to completely bypass udev due to the amount of issues
the umh approach had.
The usermode helper locks were kept even when the direct filesystem lookup
mechanism is used though. A lot has changed since the original usermode
helper locks were added but the recent commit which added the code for
firmware_enabled() are intended to address any possible races cured only
as collateral by using the locks as though side consequence of code
evolution and this not being addressed any time sooner. With the
firmware_enabled() code in place we are a bit more sure to move the
usermode helper locks to UMH only code.
There is a bit of history here so let's recap a bit of it to ensure nothing
is lost and things are clear. The direct filesystem approach to loading
firmware is rather new, it was added via commit abb139e75c ("firmware:
teach the kernel to load firmware files directly from the filesystem") by
Linus merged on the v3.7 release, to enable to bypass udev.
usermodehelper_read_lock_wait() was added earlier via commit 9b78c1da60
("firmware_class: Do not warn that system is not ready from async loads")
merged on v3.4, after Rafael noted that the async firmware API call
request_firmware_nowait() should not be penalized to fail if userspace is
not available yet or frozen, it'd allow for a timeout grace period before
giving up. The WARN_ON() was kept for the sync firmware API call though on
request_firmware(). At this time there was no direct filesystem lookup for
firmware though.
The original usermode helper lock came from commit a144c6a6c9 ("PM:
Print a warning if firmware is requested when tasks are frozen") merged on
the v3.0 kernel by Rafael to print a warning back when firmware requests
were used on resume(), thaw() or restore() callbacks and there was no
direct fs lookups or the firmware cache.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This will make subsequent changes easier to read.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The firmware API should not be used after we go to suspend
and after we reboot/halt. The suspend/resume case is a bit
complex, so this documents that so things are clearer.
We want to know about users of the API in incorrect places so
that their callers are corrected, so this also adds a warn
for those cases.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that we've have proper wrappers for the fallback mechanism
we can easily share the reboot notifier for the firmware_class
at all times.
This change will make subsequent modifications to the reboot
notifier easier to review.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We kill pending fallback requests on suspend and reboot,
the only difference is that on suspend we only kill custom
fallback requests. Provide a wrapper that lets us customize
the request with a flag.
This also lets us simplify the #ifdef'ery over the calls.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This routine will used in functions declared earlier next. This
code shift has no functional changes, it will make subsequent
changes easier to read.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since commit 5d47ec02c3 ("firmware: Correct handling of
fw_state_wait() return value") fw_load_abort() could be called twice and
lead us to a kernel crash. This happens only when the firmware fallback
mechanism (regular or custom) is used. The fallback mechanism exposes a
sysfs interface for userspace to upload a file and notify the kernel
when the file is loaded and ready, or to cancel an upload by echo'ing -1
into on the loading file:
echo -n "-1" > /sys/$DEVPATH/loading
This will call fw_load_abort(). Some distributions actually have a udev
rule in place to *always* immediately cancel all firmware fallback
mechanism requests (Debian), they have:
$ cat /lib/udev/rules.d/50-firmware.rules
# stub for immediately telling the kernel that userspace firmware loading
# failed; necessary to avoid long timeouts with CONFIG_FW_LOADER_USER_HELPER=y
SUBSYSTEM=="firmware", ACTION=="add", ATTR{loading}="-1
Distributions with this udev rule would run into this crash only if the
fallback mechanism is used. Since most distributions disable by default
using the fallback mechanism (CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
this would typicaly mean only 2 drivers which *require* the fallback
mechanism could typically incur a crash: drivers/firmware/dell_rbu.c and
the drivers/leds/leds-lp55xx-common.c driver. Distributions enabling
CONFIG_FW_LOADER_USER_HELPER_FALLBACK by default are obviously more
exposed to this crash.
The crash happens because after commit 5b02962494 ("firmware: do not
use fw_lock for fw_state protection") and subsequent fix commit
5d47ec02c3 ("firmware: Correct handling of fw_state_wait() return
value") a race can happen between this cancelation and the firmware
fw_state_wait_timeout() being woken up after a state change with which
fw_load_abort() as that calls swake_up(). Upon error
fw_state_wait_timeout() will also again call fw_load_abort() and trigger
a null reference.
At first glance we could just fix this with a !buf check on
fw_load_abort() before accessing buf->fw_st, however there is a logical
issue in having a state machine used for the fallback mechanism and
preventing access from it once we abort as its inside the buf
(buf->fw_st).
The firmware_class.c code is setting the buf to NULL to annotate an
abort has occurred. Replace this mechanism by simply using the state
check instead. All the other code in place already uses similar checks
for aborting as well so no further changes are needed.
An oops can be reproduced with the new fw_fallback.sh fallback mechanism
cancellation test. Either cancelling the fallback mechanism or the
custom fallback mechanism triggers a crash.
mcgrof@piggy ~/linux-next/tools/testing/selftests/firmware
(git::20170111-fw-fixes)$ sudo ./fw_fallback.sh
./fw_fallback.sh: timeout works
./fw_fallback.sh: firmware comparison works
./fw_fallback.sh: fallback mechanism works
[ this then sits here when it is trying the cancellation test ]
Kernel log:
test_firmware: loading 'nope-test-firmware.bin'
misc test_firmware: Direct firmware load for nope-test-firmware.bin failed with error -2
misc test_firmware: Falling back to user helper
BUG: unable to handle kernel NULL pointer dereference at 0000000000000038
IP: _request_firmware+0xa27/0xad0
PGD 0
Oops: 0000 [#1] SMP
Modules linked in: test_firmware(E) ... etc ...
CPU: 1 PID: 1396 Comm: fw_fallback.sh Tainted: G W E 4.10.0-rc3-next-20170111+ #30
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.1-0-g8891697-prebuilt.qemu-project.org 04/01/2014
task: ffff9740b27f4340 task.stack: ffffbb15c0bc8000
RIP: 0010:_request_firmware+0xa27/0xad0
RSP: 0018:ffffbb15c0bcbd10 EFLAGS: 00010246
RAX: 00000000fffffffe RBX: ffff9740afe5aa80 RCX: 0000000000000000
RDX: ffff9740b27f4340 RSI: 0000000000000283 RDI: 0000000000000000
RBP: ffffbb15c0bcbd90 R08: ffffbb15c0bcbcd8 R09: 0000000000000000
R10: 0000000894a0d4b1 R11: 000000000000008c R12: ffffffffc0312480
R13: 0000000000000005 R14: ffff9740b1c32400 R15: 00000000000003e8
FS: 00007f8604422700(0000) GS:ffff9740bfc80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000038 CR3: 000000012164c000 CR4: 00000000000006e0
Call Trace:
request_firmware+0x37/0x50
trigger_request_store+0x79/0xd0 [test_firmware]
dev_attr_store+0x18/0x30
sysfs_kf_write+0x37/0x40
kernfs_fop_write+0x110/0x1a0
__vfs_write+0x37/0x160
? _cond_resched+0x1a/0x50
vfs_write+0xb5/0x1a0
SyS_write+0x55/0xc0
? trace_do_page_fault+0x37/0xd0
entry_SYSCALL_64_fastpath+0x1e/0xad
RIP: 0033:0x7f8603f49620
RSP: 002b:00007fff6287b788 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 000055c307b110a0 RCX: 00007f8603f49620
RDX: 0000000000000016 RSI: 000055c3084d8a90 RDI: 0000000000000001
RBP: 0000000000000016 R08: 000000000000c0ff R09: 000055c3084d6336
R10: 000055c307b108b0 R11: 0000000000000246 R12: 000055c307b13c80
R13: 000055c3084d6320 R14: 0000000000000000 R15: 00007fff6287b950
Code: 9f 64 84 e8 9c 61 fe ff b8 f4 ff ff ff e9 6b f9 ff
ff 48 c7 c7 40 6b 8d 84 89 45 a8 e8 43 84 18 00 49 8b be 00 03 00 00 8b
45 a8 <83> 7f 38 02 74 08 e8 6e ec ff ff 8b 45 a8 49 c7 86 00 03 00 00
RIP: _request_firmware+0xa27/0xad0 RSP: ffffbb15c0bcbd10
CR2: 0000000000000038
---[ end trace 6d94ac339c133e6f ]---
Fixes: 5d47ec02c3 ("firmware: Correct handling of fw_state_wait() return value")
Reported-and-Tested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reported-and-Tested-by: Patrick Bruenn <p.bruenn@beckhoff.com>
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
CC: <stable@vger.kernel.org> [3.10+]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When request_firmware() finds an already open firmware object it will
wait for that object to become fully loaded and then check the status.
As __fw_state_wait_common() succeeds the timeout value returned will be
truncated in _request_firmware_prepare() and interpreted as -EPERM.
Prior to "firmware: do not use fw_lock for fw_state protection" the code
did test if we where in the "done" state before sleeping, causing this
particular code path to succeed, in some cases.
As the callers are interested in the result of the wait and not the
remaining timeout the return value of __fw_state_wait_common() is
changed to signal "done" or "error", which simplifies the logic in
_request_firmware_load() as well.
Fixes: 5b02962494 ("firmware: do not use fw_lock for fw_state protection")
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch removes following error at for `make htmldocs`. No functional
change.
./drivers/base/firmware_class.c:1348: WARNING: Bullet list ends without a blank line; unexpected unindent.
Signed-off-by: Silvio Fricke <silvio.fricke@gmail.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fw_state_is_done() is only used for UHM so moved into that section.
Cc: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fw_lock is to use to protect 'corner cases' inside firmware_class. It
is not exactly clear what those corner cases are nor what it exactly
protects. fw_state can be used without needing the fw_lock to protect
its state transition and wake ups.
fw_state is holds the state in status and the completion is used to
wake up all waiters (in this case that is the user land helper so only
one). This operation has to be 'atomic' to avoid races. We can do this
by using swait which takes care we don't miss any wake up.
We use also swait instead of wait because don't need all the additional
features wait provides.
Note there some more cleanups possible after with this change. For
example for !CONFIG_FW_LOADER_USER_HELPER we don't check for the state
anymore. Let's to this in the next patch instead mingling to many
changes into this one. And yes you get a gcc warning "‘__fw_state_check’
defined but not used [-Wunused-function] code." for the time beeing.
Cc: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We track the state of the firmware loading with bit ops. Since the
state machine has only a few states and they are all mutual exclusive
there are only a few simple state transition we can model this simplify.
UNKNOWN -> LOADING -> DONE | ABORTED
Because we don't use any bit ops on fw_state::status anymore we are able
to change the data type to enum fw_status and update the function
arguments accordingly.
READ_ONCE() and WRITE_ONCE() are propably not needed because there are a
lot of load and stores around fw_st->status. But let's make it explicit
and not be sorry later.
Cc: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The firmware loader tracks the current state of the loading process
via unsigned long status and a completion in struct
firmware_buf. Instead of open code tracking the state, introduce data
structure which encapsulate the state tracking and synchronization.
While at it also separate UHM states from direct loading states, e.g.
the loading_timeout is only defined when CONFIG_FW_LOADER_USER_HELPER.
Cc: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When you use the firmware usermode helper fallback with a timeout value set to a
value greater than INT_MAX (2147483647) a cast overflow issue causes the
timeout value to go negative and breaks all usermode helper loading. This
regression was introduced through commit 68ff2a00db ("firmware_loader:
handle timeout via wait_for_completion_interruptible_timeout()") on kernel
v4.0.
The firmware_class drivers relies on the firmware usermode helper
fallback as a mechanism to look for firmware if the direct filesystem
search failed only if:
a) You've enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK (not many distros):
Then all of these callers will rely on the fallback mechanism in case
the firmware is not found through an initial direct filesystem lookup:
o request_firmware()
o request_firmware_into_buf()
o request_firmware_nowait()
b) If you've only enabled CONFIG_FW_LOADER_USER_HELPER (most distros):
Then only callers using request_firmware_nowait() with the second
argument set to false, this explicitly is requesting the UMH firmware
fallback to be relied on in case the first filesystem lookup fails.
Using Coccinelle SmPL grammar we have identified only two drivers
explicitly requesting the UMH firmware fallback mechanism:
- drivers/firmware/dell_rbu.c
- drivers/leds/leds-lp55xx-common.c
Since most distributions only enable CONFIG_FW_LOADER_USER_HELPER the
biggest impact of this regression are users of the dell_rbu and
leds-lp55xx-common device driver which required the UMH to find their
respective needed firmwares.
The default timeout for the UMH is set to 60 seconds always, as of
commit 68ff2a00db ("firmware_loader: handle timeout via
wait_for_completion_interruptible_timeout()") the timeout was bumped
to MAX_JIFFY_OFFSET ((LONG_MAX >> 1)-1). Additionally the MAX_JIFFY_OFFSET
value was also used if the timeout was configured by a user to 0.
The following works:
echo 2147483647 > /sys/class/firmware/timeout
But both of the following set the timeout to MAX_JIFFY_OFFSET even if
we display 0 back to userspace:
echo 2147483648 > /sys/class/firmware/timeout
cat /sys/class/firmware/timeout
0
echo 0> /sys/class/firmware/timeout
cat /sys/class/firmware/timeout
0
A max value of INT_MAX (2147483647) seconds is therefore implicit due to the
another cast with simple_strtol().
This fixes the secondary cast (the first one is simple_strtol() but its an
issue only by forcing an implicit limit) by re-using the timeout variable and
only setting retval in appropriate cases.
Lastly worth noting systemd had ripped out the UMH firmware fallback
mechanism from udev since udev 2014 via commit be2ea723b1d023b3d
("udev: remove userspace firmware loading support"), so as of systemd v217.
Signed-off-by: Yves-Alexis Perez <corsac@corsac.net>
Fixes: 68ff2a00db "firmware_loader: handle timeout via wait_for_completion_interruptible_timeout()"
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable@vger.kernel.org
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[mcgrof@kernel.org: gave commit log a whole lot of love]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Convert the firmware core to use class_groups instead of class_attrs as
that's the correct way to handle lists of class attribute files.
Cc: Ming Lei <ming.lei@canonical.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Some systems are memory constrained but they need to load very large
firmwares. The firmware subsystem allows drivers to request this
firmware be loaded from the filesystem, but this requires that the
entire firmware be loaded into kernel memory first before it's provided
to the driver. This can lead to a situation where we map the firmware
twice, once to load the firmware into kernel memory and once to copy the
firmware into the final resting place.
This creates needless memory pressure and delays loading because we have
to copy from kernel memory to somewhere else. Let's add a
request_firmware_into_buf() API that allows drivers to request firmware
be loaded directly into a pre-allocated buffer. This skips the
intermediate step of allocating a buffer in kernel memory to hold the
firmware image while it's read from the filesystem. It also requires
that drivers know how much memory they'll require before requesting the
firmware and negates any benefits of firmware caching because the
firmware layer doesn't manage the buffer lifetime.
For a 16MB buffer, about half the time is spent performing a memcpy from
the buffer to the final resting place. I see loading times go from
0.081171 seconds to 0.047696 seconds after applying this patch. Plus
the vmalloc pressure is reduced.
This is based on a patch from Vikram Mulukutla on codeaurora.org:
https://www.codeaurora.org/cgit/quic/la/kernel/msm-3.18/commit/drivers/base/firmware_class.c?h=rel/msm-3.18&id=0a328c5f6cd999f5c591f172216835636f39bcb5
Link: http://lkml.kernel.org/r/20160607164741.31849-4-stephen.boyd@linaro.org
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Vikram Mulukutla <markivx@codeaurora.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Some low memory systems with complex peripherals cannot afford to have
the relatively large firmware images taking up valuable memory during
suspend and resume. Change the internal implementation of
firmware_class to disallow caching based on a configurable option. In
the near future, variants of request_firmware will take advantage of
this feature.
Link: http://lkml.kernel.org/r/20160607164741.31849-3-stephen.boyd@linaro.org
[stephen.boyd@linaro.org: Drop firmware_desc design and use flags]
Signed-off-by: Vikram Mulukutla <markivx@codeaurora.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Some systems are memory constrained but they need to load very large
firmwares. The firmware subsystem allows drivers to request this
firmware be loaded from the filesystem, but this requires that the
entire firmware be loaded into kernel memory first before it's provided
to the driver. This can lead to a situation where we map the firmware
twice, once to load the firmware into kernel memory and once to copy the
firmware into the final resting place.
This design creates needless memory pressure and delays loading because
we have to copy from kernel memory to somewhere else. This patch sets
adds support to the request firmware API to load the firmware directly
into a pre-allocated buffer, skipping the intermediate copying step and
alleviating memory pressure during firmware loading. The drawback is
that we can't use the firmware caching feature because the memory for
the firmware cache is not managed by the firmware layer.
This patch (of 3):
We use similar structured code to read and write the kmapped firmware
pages. The only difference is read copies from the kmap region and
write copies to it. Consolidate this into one function to reduce
duplication.
Link: http://lkml.kernel.org/r/20160607164741.31849-2-stephen.boyd@linaro.org
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
Cc: Vikram Mulukutla <markivx@codeaurora.org>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Here is the big char/misc driver update for 4.6-rc1.
The majority of the patches here is hwtracing and some new mic drivers,
but there's a lot of other driver updates as well. Full details in the
shortlog.
All have been in linux-next for a while with no reported issues.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iEYEABECAAYFAlbp9IcACgkQMUfUDdst+ykyJgCeLTC2QNGrh51kiJglkVJ0yD36
q4MAn0NkvSX2+iv5Jq8MaX6UQoRa4Nun
=MNjR
-----END PGP SIGNATURE-----
Merge tag 'char-misc-4.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc
Pull char/misc updates from Greg KH:
"Here is the big char/misc driver update for 4.6-rc1.
The majority of the patches here is hwtracing and some new mic
drivers, but there's a lot of other driver updates as well. Full
details in the shortlog.
All have been in linux-next for a while with no reported issues"
* tag 'char-misc-4.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc: (238 commits)
goldfish: Fix build error of missing ioremap on UM
nvmem: mediatek: Fix later provider initialization
nvmem: imx-ocotp: Fix return value of imx_ocotp_read
nvmem: Fix dependencies for !HAS_IOMEM archs
char: genrtc: replace blacklist with whitelist
drivers/hwtracing: make coresight-etm-perf.c explicitly non-modular
drivers: char: mem: fix IS_ERROR_VALUE usage
char: xillybus: Fix internal data structure initialization
pch_phub: return -ENODATA if ROM can't be mapped
Drivers: hv: vmbus: Support kexec on ws2012 r2 and above
Drivers: hv: vmbus: Support handling messages on multiple CPUs
Drivers: hv: utils: Remove util transport handler from list if registration fails
Drivers: hv: util: Pass the channel information during the init call
Drivers: hv: vmbus: avoid unneeded compiler optimizations in vmbus_wait_for_unload()
Drivers: hv: vmbus: remove code duplication in message handling
Drivers: hv: vmbus: avoid wait_for_completion() on crash
Drivers: hv: vmbus: don't loose HVMSG_TIMER_EXPIRED messages
misc: at24: replace memory_accessor with nvmem_device_read
eeprom: 93xx46: extend driver to plug into the NVMEM framework
eeprom: at25: extend driver to plug into the NVMEM framework
...
When we now use the new kernel_read_file_from_path() we
are reporting a failure when we iterate over all the paths
possible for firmware. Before using kernel_read_file_from_path()
we only reported a failure once we confirmed a file existed
with filp_open() but failed with fw_read_file_contents().
With kernel_read_file_from_path() both are done for us and
we obviously are now reporting too much information given that
some optional paths will always fail and clutter the logs.
fw_get_filesystem_firmware() already has a check for failure
and uses an internal flag, FW_OPT_NO_WARN, but this does not
let us capture other unxpected errors. This enables that
as changed by Neil via commit:
"firmware: Be a bit more verbose about direct firmware loading failure"
Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
Replace the fw_read_file_contents with kernel_file_read_from_path().
Although none of the upstreamed LSMs define a kernel_fw_from_file hook,
IMA is called by the security function to prevent unsigned firmware from
being loaded and to measure/appraise signed firmware, based on policy.
Instead of reading the firmware twice, once for measuring/appraising the
firmware and again for reading the firmware contents into memory, the
kernel_post_read_file() security hook calculates the file hash based on
the in memory file buffer. The firmware is read once.
This patch removes the LSM kernel_fw_from_file() hook and security call.
Changelog v4+:
- revert dropped buf->size assignment - reported by Sergey Senozhatsky
v3:
- remove kernel_fw_from_file hook
- use kernel_file_read_from_path() - requested by Luis
v2:
- reordered and squashed firmware patches
- fix MAX firmware size (Kees Cook)
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
This makes the error and success paths more readable while trying to
load firmware from the filesystem.
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: David Howells <dhowells@redhat.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
This will be re-used later through a new extensible interface.
Reviewed-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Simplify a few of the *generic* shared dev_warn() and dev_dbg()
print messages for three reasons:
0) Historically firmware_class code was added to help
get device driver firmware binaries but these days
request_firmware*() helpers are being repurposed for
general *system data* needed by the kernel.
1) This will also help generalize shared code as much as possible
later in the future in consideration for a new extensible firmware
API which will enable to separate usermode helper code out as much
as possible.
2) Kees Cook pointed out the the prints already have the device
associated as dev_*() helpers are used, that should help identify
the user and case in which the helpers are used. That should provide
enough context and simplifies the messages further.
v4: generalize debug/warn messages even further as suggested by
Kees Cook.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Vojtěch Pavlík <vojtech@suse.cz>
Cc: Kyle McMartin <kyle@kernel.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
No need to use use continuous memory, it may be fail
when memory deeply fragmented.
Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Xia Qing <saberlily.xia@hisilicon.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Simplify a few of the *generic* shared dev_warn() and dev_dbg()
print messages for three reasons:
0) Historically firmware_class code was added to help
get device driver firmware binaries but these days
request_firmware*() helpers are being repurposed for
general *system data* needed by the kernel.
1) This will also help generalize shared code as much as possible
later in the future in consideration for a new extensible firmware
API which will enable to separate usermode helper code out as much
as possible.
2) Kees Cook pointed out the the prints already have the device
associated as dev_*() helpers are used, that should help identify
the user and case in which the helpers are used. That should provide
enough context and simplifies the messages further.
v4: generalize debug/warn messages even further as suggested by
Kees Cook.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Vojtěch Pavlík <vojtech@suse.cz>
Cc: Kyle McMartin <kyle@kernel.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Device resource data allocated with devres_alloc() must be deallocated
by devres_free().
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Acked-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The firmware class uevent function accessed the "fw_priv->buf" buffer
without the proper locking and testing for NULL. This is an old bug
(looks like it goes back to 2012 and commit 1244691c73b2: "firmware
loader: introduce firmware_buf"), but for some reason it's triggering
only now in 4.2-rc1.
Shuah Khan is trying to bisect what it is that causes this to trigger
more easily, but in the meantime let's just fix the bug since others are
hitting it too (at least Ingo reports having seen it as well).
Reported-and-tested-by: Shuah Khan <shuahkh@osg.samsung.com>
Acked-by: Ming Lei <ming.lei@canonical.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The recent fix to use kstrdup_const() failed to add a
kfree upon failure of name allocation...
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Kyle McMartin <kyle@kernel.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We currently use flexible arrays with a char at the
end for the remaining internal firmware name uses.
There are two limitations with the way we use this.
Since we're using a flexible array for a string on the
struct if we wanted to use two strings it means we'd
have a disjoint means of handling the strings, one
using the flexible array, and another a char * pointer.
We're also currently not using 'const' for the string.
We wish to later extend some firmware data structures
with other string/char pointers, but we also want to be
very pedantic about const usage. Since we're going to
change things to use 'const' we might as well also address
unified way to use multiple strings on the structs.
Replace the flexible array practice for strings with
kstrdup_const() and kfree_const(), this will avoid
allocations when the vmlinux .rodata is used, and just
allocate a new proper string for us when needed. This
also means we can simplify the struct allocations by
removing the string length from the allocation size
computation, which would otherwise get even more
complicated when supporting multiple strings.
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Howells <dhowells@redhat.com>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Kyle McMartin <kyle@kernel.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When direct firmware loading is used we iterate over a list
of possible firmware paths and concatenate the desired firmware
name with each path and look for the file there. Should the
passed firmware name be too long we end up truncating the
file we want to look for, the search however is still done.
Add a check for truncation instead of looking for a
truncated firmware filename.
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Howells <dhowells@redhat.com>
Cc: Kyle McMartin <kyle@kernel.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The request_firmware*() APIs uses __getname() to iterate
over the list of paths possible for firmware to be found,
the code however never checked for failure on __getname().
Although *very unlikely*, this can still happen. Add the
missing check.
There is still no checks on the concatenation of the path
and filename passed, that requires a bit more work and
subsequent patches address this. The commit that introduced
this is abb139e7 ("firmware: teach the kernel to load
firmware files directly from the filesystem").
mcgrof@ergon ~/linux (git::firmware-fixes) $ git describe --contains abb139e7
v3.7-rc1~120
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Howells <dhowells@redhat.com>
Cc: Kyle McMartin <kyle@kernel.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When using the user mode helper to load firmwares the function _request_firmware
gets a positive return value from fw_load_from_user_helper and because of this
the firmware buffer is not assigned. This happens only when the return value
is zero. This patch fixes this problem in _request_firmware_load. When the
completion is ready the return value is set to zero.
Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
Cc: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use the static attribute groups assigned to the device instead of
manual device_create_file() & co calls. It simplifies the code and
can avoid possible races, too.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Really tiny set of patches for this kernel. Nothing major, all
described in the shortlog and have been in linux-next for a while.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iEYEABECAAYFAlTgtIAACgkQMUfUDdst+ymjSwCfWspNT71lmsVwasCTPQopgXov
TqAAoKR4I5ZebMks/nW6ClxUFYwVSL02
=leVc
-----END PGP SIGNATURE-----
Merge tag 'driver-core-3.20-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core
Pull driver core patches from Greg KH:
"Really tiny set of patches for this kernel. Nothing major, all
described in the shortlog and have been in linux-next for a while"
* tag 'driver-core-3.20-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core:
sysfs: fix warning when creating a sysfs group without attributes
firmware_loader: handle timeout via wait_for_completion_interruptible_timeout()
firmware_loader: abort request if wait_for_completion is interrupted
firmware: Correct function name in comment
device: Change dev_<level> logging functions to return void
device: Fix dev_dbg_once macro
This patch reduces the kernel size by removing error messages that duplicate
the normal OOM message.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)
@@
identifier f,print,l;
expression e;
constant char[] c;
@@
e = \(kzalloc\|kmalloc\|devm_kzalloc\|devm_kmalloc\)(...);
if (e == NULL) {
<+...
- print(...,c,...);
... when any
(
goto l;
|
return ...;
)
...+> }
Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
Acked-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
It is simpler to handle timeout by wait_for_completion_interruptible_timeout(),
so remove previous delay work for timeout.
Signed-off-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If current request is interrupted by signal, such as 'ctrl + c',
this request has to be aborted for the following reasons:
- the buf need to be removed from pending list
- same requests from other contexts need to be completed
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use the correct function name in the kernel-doc comment above it.
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The vunmap() function performes also input parameter validation. Thus the test
around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
An empty firmware request name will trigger warnings when building
device names. Make sure this is caught earlier and rejected.
The warning was visible via the test_firmware.ko module interface:
echo -ne "\x00" > /sys/devices/virtual/misc/test_firmware/trigger_request
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Tested-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Pull security subsystem updates from James Morris:
"In this release:
- PKCS#7 parser for the key management subsystem from David Howells
- appoint Kees Cook as seccomp maintainer
- bugfixes and general maintenance across the subsystem"
* 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (94 commits)
X.509: Need to export x509_request_asymmetric_key()
netlabel: shorter names for the NetLabel catmap funcs/structs
netlabel: fix the catmap walking functions
netlabel: fix the horribly broken catmap functions
netlabel: fix a problem when setting bits below the previously lowest bit
PKCS#7: X.509 certificate issuer and subject are mandatory fields in the ASN.1
tpm: simplify code by using %*phN specifier
tpm: Provide a generic means to override the chip returned timeouts
tpm: missing tpm_chip_put in tpm_get_random()
tpm: Properly clean sysfs entries in error path
tpm: Add missing tpm_do_selftest to ST33 I2C driver
PKCS#7: Use x509_request_asymmetric_key()
Revert "selinux: fix the default socket labeling in sock_graft()"
X.509: x509_request_asymmetric_keys() doesn't need string length arguments
PKCS#7: fix sparse non static symbol warning
KEYS: revert encrypted key change
ima: add support for measuring and appraising firmware
firmware_class: perform new LSM checks
security: introduce kernel_fw_from_file hook
PKCS#7: Missing inclusion of linux/err.h
...
This attaches LSM hooks to the existing firmware loading interfaces:
filesystem-found firmware and demand-loaded blobs. On errors, loads
are aborted and the failure code is returned to userspace.
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
_request_firmware_load() returns -ENOMEM when fw load is aborted after
timeout. Call is_fw_load_aborted() to check if fw load is aborted and
if true return -EAGAIN.
Signed-off-by: Shuah Khan <shuah.kh@samsung.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Now that the udev firmware loader is optional request_firmware()
will not provide any information on the kernel ring buffer if
direct firmware loading failed and udev firmware loading is disabled.
If no information is needed request_firmware_direct() should be used
for optional firmware, at which point drivers can take on the onus
over informing of any failures, if udev firmware loading is disabled
though we should at the very least provide some sort of information
as when the udev loader was enabled by default back in the days.
With this change with a simple firmware load test module [0]:
Example output without FW_LOADER_USER_HELPER_FALLBACK
platform fake-dev.0: Direct firmware load for fake.bin failed
with error -2
Example with FW_LOADER_USER_HELPER_FALLBACK
platform fake-dev.0: Direct firmware load for fake.bin failed with error -2
platform fake-dev.0: Falling back to user helper
Without this change without FW_LOADER_USER_HELPER_FALLBACK we
get no output logged upon failure.
Cc: Tom Gundersen <teg@jklm.no>
Cc: Ming Lei <ming.lei@canonical.com>
Cc: Abhay Salunke <Abhay_Salunke@dell.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Kay Sievers <kay@vrfy.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
use mm.h definition
Cc: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Luis R. Rodriguez
Bjorn Andersson
Silvio Fricke
Daniel Wagner
Yves-Alexis Perez
Greg Kroah-Hartman
Stephen Boyd
Vikram Mulukutla
Linus Torvalds
Luis R. Rodriguez
Mimi Zohar
Kees Cook
Chen Feng
Brian Norris
Vladimir Zapolskiy
Zahari Doychev
Takashi Iwai
Andrei Oprea
Quentin Lambert
Ming Lei
Borislav Petkov
Markus Elfring
Kweh, Hock Leong
Shuah Khan
Fabian Frederick