On Wed, Jun 14, 2017 at 03:20:13PM -0700, Luis R. Rodriguez wrote: > Martin reported an issue with Android where if sysfs is used to trigger a sync > fw load which *relies* on the fallback mechanism and a background job completes > while the trigger is ongoing in the foreground it will immediately fail the fw > request. The issue can be observed in this simple test script using the > test_firmware driver: > > set -e > /etc/init.d/udev stop > modprobe test_firmware > DIR=/sys/devices/virtual/misc/test_firmware > echo 10 >/sys/class/firmware/timeout > sleep 2 & > echo -n "does-not-exist-file.bin" > "$DIR"/trigger_request > > The background sleep triggers the SIGCHLD signal and we fail the firmware > request on the fallback mechanism. This was due to the type of wait used which > captures all signals, but we currently lack the killable swaits which would > only allow killing the fallback wait on SIGKILL. This adds the missing killable > swaits and fixes the firmware API to use it. This goes along with a test case to > demo the issue clearly and how its fixed afterwards. Lastly, ensure to use > -EINTR when interrupted so callers can distinguish between an interrupted > failure and other types of failure. > > As suggested I've tagged the addition of the killable swaits and the firmware > fix as stable. Between v4.0 and v4.10 the stable fix is to instead change > the firmware to use wait_for_completion_killable_timeout() as the firmware > API was only converted to swait as of v4.10. > > The last patch must be applied only after the killable wait is applied to > ensure only SIGKILL triggers an interruption. Otherwise it has been observed > using -EINTR on other signals like SIGCHLD will trigger a restart of the call, > if a sysfs write() is used to trigger the sync firmware request. This might be > due what signal(7) says about using the SA_RESTART flag when write() is called, > in such cases the call will be automatically restarted after the signal handler > returns. The excemption here naturally seems to be on SIGKILL. > > Note that although I *feared* this might implicate any use of non-killable waits > on other system calls, such as finit_module(), initial testing confirms this to > not be the case. For instance replacing the echo with modprobe on a module > which does the same on init does not present the same issues. This could be due > to the special SA_RESTART flag case on write() as noted above and sysfs... > however, its not perfectly clear yet to me. > > As usual these patches are on my linux-next git tree, they are on the > 20170614-fw-fixes branch based on linux-next 20170614 [0]. > > [0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170614-fw-fixes Greg, *poke* Luis
