* [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback
[not found] <20170524205658.GK8951@wotan.suse.de>
@ 2017-05-24 21:40 ` Luis R. Rodriguez
2017-05-24 22:00 ` Andy Lutomirski
0 siblings, 1 reply; 2+ messages in thread
From: Luis R. Rodriguez @ 2017-05-24 21:40 UTC (permalink / raw)
To: gregkh
Cc: wagi, dwmw2, jewalt, rafal, arend.vanspriel, rjw, yi1.li, atull,
moritz.fischer, pmladek, johannes.berg, emmanuel.grumbach,
luciano.coelho, kvalo, luto, torvalds, keescook, takahiro.akashi,
dhowells, pjones, hdegoede, alan, tytso, linux-kernel,
Martin Fuzzey, stable # 4 . 0, Luis R . Rodriguez
From: Martin Fuzzey <mfuzzey@parkeon.com>
Commit 0cb64249ca500 ("firmware_loader: abort request if wait_for_completion
is interrupted") added via 4.0 added support to abort the fallback mechanism
when a signal was detected and wait_for_completion_interruptible() returned
-ERESTARTSYS. Although the abort was effective we were unfortunately never
really propagating this error though and as such userspace could not know
why the abort happened.
The error code was always being lost to an even older change, commit
0542ad88fbdd81bb ("firmware loader: Fix _request_firmware_load() return val
for fw load abort") by Shuah Khan which was merged since v3.17. Back then
though we never were capturing these signals or bailing on a signal. After
this change though only only -EAGAIN was being relayed back to userspace
on non-memory errors including signals trying to interrupt our fallback
process.
It only makes sense to fix capturing -ERESTARTSYS since we were capturing
the error but when it was actually effective, since commit 0cb64249ca500
("firmware_loader: abort request if wait_for_completion is interrupted").
Only distributions relying on the fallback mechanism are impacted. An
example issue is on Android, when request_firmware() is called through
the firmware fallback mechanism -- syfs write call, sysfs .store(), and
Android sends a SIGCHLD to fail the write call -- in such cases the fact
that we failed due to a signal is lost.
Fix this and ensure we propagate -ERESTARTSYS so that handlers can
whether or not to restart write calls.
Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
Cc: stable <stable@vger.kernel.org> # 4.0
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: gave the commit log some serious love]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
For those following drivers/base/firmware_class.c development -- I've pushed
out two new branches based on linux-next tag next-20170524, one is just the
driver-data API [0], and the other one just has this patch applied on top
of the driver-data API [1]. You'll want to use driver-data-stable if you are
working on the cache or the fallback mechanism since this fix does provide
a fix for the cache / fallback mechanism.
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170524-driver-data
https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170524-driver-data-stable
Luis
drivers/base/firmware_class.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 7af430a2d656..77c0e0792c30 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -1309,9 +1309,10 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
mutex_unlock(&fw_lock);
}
- if (fw_state_is_aborted(&buf->fw_st))
- retval = -EAGAIN;
- else if (buf->is_paged_buf && !buf->data)
+ if (fw_state_is_aborted(&buf->fw_st)) {
+ if (retval != -ERESTARTSYS)
+ retval = -EAGAIN;
+ } else if (buf->is_paged_buf && !buf->data)
retval = -ENOMEM;
device_del(f_dev);
--
2.10.2
^ permalink raw reply related [flat|nested] 2+ messages in thread