linux-kernel - Re: [PATCH 0/4] firmware: fix fallback mechanism by ignoring SIGCHLD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 26 Jun 2017 23:19:04 +0200
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     "Luis R. Rodriguez" <mcgrof@...nel.org>
Cc:     gregkh@...uxfoundation.org, mfuzzey@...keon.com,
        ebiederm@...ssion.com, dmitry.torokhov@...il.com, wagi@...om.org,
        dwmw2@...radead.org, jewalt@...innovations.com, rafal@...ecki.pl,
        arend.vanspriel@...adcom.com, rjw@...ysocki.net,
        yi1.li@...ux.intel.com, atull@...nel.org, moritz.fischer@...us.com,
        pmladek@...e.com, johannes.berg@...el.com,
        emmanuel.grumbach@...el.com, luciano.coelho@...el.com,
        kvalo@...eaurora.org, luto@...nel.org,
        torvalds@...ux-foundation.org, keescook@...omium.org,
        takahiro.akashi@...aro.org, dhowells@...hat.com, pjones@...hat.com,
        hdegoede@...hat.com, alan@...ux.intel.com, tytso@....edu,
        mtk.manpages@...il.com, paul.gortmaker@...driver.com,
        mtosatti@...hat.com, mawilcox@...rosoft.com,
        linux-api@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/4] firmware: fix fallback mechanism by ignoring SIGCHLD

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