linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <mcgrof@kernel.org>
To: gregkh@linuxfoundation.org
Cc: wagi@monom.org, dwmw2@infradead.org, jewalt@lgsinnovations.com,
	rafal@milecki.pl, arend.vanspriel@broadcom.com,
	rjw@rjwysocki.net, yi1.li@linux.intel.com,
	atull@opensource.altera.com, moritz.fischer@ettus.com,
	pmladek@suse.com, johannes.berg@intel.com,
	emmanuel.grumbach@intel.com, luciano.coelho@intel.com,
	kvalo@codeaurora.org, luto@kernel.org,
	torvalds@linux-foundation.org, keescook@chromium.org,
	takahiro.akashi@linaro.org, dhowells@redhat.com,
	pjones@redhat.com, hdegoede@redhat.com, alan@linux.intel.com,
	tytso@mit.edu, linux-kernel@vger.kernel.org,
	Martin Fuzzey <mfuzzey@parkeon.com>,
	"stable # 4 . 0" <stable@vger.kernel.org>,
	"Luis R . Rodriguez" <mcgrof@kernel.org>
Subject: [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback
Date: Wed, 24 May 2017 14:40:27 -0700	[thread overview]
Message-ID: <20170524214027.7775-1-mcgrof@kernel.org> (raw)
In-Reply-To: <20170524205658.GK8951@wotan.suse.de>

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

  reply	other threads:[~2017-05-24 21:40 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-23 13:16 [PATCH] firmware: request_firmware() should propagate -ERESTARTSYS Martin Fuzzey
2017-05-23 13:31 ` Greg Kroah-Hartman
2017-05-23 14:32   ` Martin Fuzzey
2017-05-23 19:55     ` Luis R. Rodriguez
2017-05-24 20:56       ` Luis R. Rodriguez
2017-05-24 21:40         ` Luis R. Rodriguez [this message]
2017-05-24 22:00           ` [PATCH v2] firmware: fix sending -ERESTARTSYS due to signal on fallback Andy Lutomirski
2017-05-24 22:38             ` Luis R. Rodriguez
2017-05-25  4:13               ` Andy Lutomirski
2017-05-25  8:28                 ` Fuzzey, Martin
2017-05-26 11:09                   ` Eric W. Biederman
2017-05-26 19:46                     ` Luis R. Rodriguez
2017-05-26 21:26                       ` Dmitry Torokhov
2017-05-26 21:32                         ` Luis R. Rodriguez
2017-05-26 21:55                           ` Dmitry Torokhov
2017-06-05 20:24                             ` Luis R. Rodriguez
2017-06-06  9:04                               ` Martin Fuzzey
2017-06-06 16:34                                 ` Luis R. Rodriguez
2017-06-06 17:52                                   ` Luis R. Rodriguez
2017-06-06 14:53                               ` Alan Cox
2017-06-06 16:47                                 ` Luis R. Rodriguez
2017-06-06 17:54                                   ` Luis R. Rodriguez
2017-06-06 22:11                                   ` Theodore Ts'o
2017-06-07  0:22                                     ` Luis R. Rodriguez
2017-06-07  4:56                                       ` Andy Lutomirski
2017-06-07  6:25                                         ` Dmitry Torokhov
2017-06-07 12:25                                           ` Alan Cox
2017-06-07 17:15                                             ` Luis R. Rodriguez
2017-06-09  1:14                                           ` Andy Lutomirski
2017-06-09  1:33                                             ` Luis R. Rodriguez
2017-06-09 21:29                                               ` Luis R. Rodriguez
2017-05-26 19:40                   ` Luis R. Rodriguez
2017-05-26 20:23                     ` Fuzzey, Martin
2017-05-26 20:52                       ` Luis R. Rodriguez
2017-06-07 17:08                   ` Luis R. Rodriguez
2017-06-07 17:54                     ` Martin Fuzzey
2017-06-09  1:10                       ` Luis R. Rodriguez
2017-06-09  1:57                         ` Luis R. Rodriguez
2017-06-09  7:40                           ` Martin Fuzzey
2017-06-09 21:12                             ` Luis R. Rodriguez
2017-06-09 22:55                             ` Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170524214027.7775-1-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=alan@linux.intel.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=atull@opensource.altera.com \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=emmanuel.grumbach@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=jewalt@lgsinnovations.com \
    --cc=johannes.berg@intel.com \
    --cc=keescook@chromium.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luciano.coelho@intel.com \
    --cc=luto@kernel.org \
    --cc=mfuzzey@parkeon.com \
    --cc=moritz.fischer@ettus.com \
    --cc=pjones@redhat.com \
    --cc=pmladek@suse.com \
    --cc=rafal@milecki.pl \
    --cc=rjw@rjwysocki.net \
    --cc=stable@vger.kernel.org \
    --cc=takahiro.akashi@linaro.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=wagi@monom.org \
    --cc=yi1.li@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).