linux-kernel - [PATCHv2] firmware: Correct handling of fw_state_wait

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-next>] [day] [month] [year] [list]

Date:   Tue, 17 Jan 2017 15:35:05 +0000
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     gregkh@...uxfoundation.org
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Daniel Wagner <daniel.wagner@...-carit.de>,
        "Luis R . Rodriguez" <mcgrof@...nel.org>,
        Ming Lei <ming.lei@...onical.com>,
        linux-kernel@...r.kernel.org, oss-drivers@...ronome.com,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: [PATCHv2] firmware: Correct handling of fw_state_wait_timeout() return value

Commit 5d47ec02c37e ("firmware: Correct handling of fw_state_wait()
return value") made the assumption that any error returned from
fw_state_wait_timeout() means FW load has to be aborted.  This is
incorrect, FW load only has to be aborted when load timed out or
has been interrupted.  Otherwise, if wait exited because of a wake
up, the waking thread (in firmware_loading_store()) had already 
performed the clean up - deleted fw_buf from the pending list and 
set fw_priv->buf to NULL.

Example NULL dereference which occurs because requsted firmware
could not be found by UMH:

nfp 0000:02:00.0: Direct firmware load for AMDA0081-0001.cat failed with error -2
nfp 0000:02:00.0: Falling back to user helper
BUG: unable to handle kernel NULL pointer dereference at 0000000000000038
IP: _request_firmware+0x7fd/0x910
PGD 0

Oops: 0000 [#1] PREEMPT SMP
CPU: 4 PID: 1981 Comm: insmod Tainted: G           O    4.10.0-rc3-perf-00404-g575a284323c2 #78
Hardware name: Dell Inc. PowerEdge T630/0W9WXC, BIOS 1.2.10 03/10/2015
task: ffff919b81a99a80 task.stack: ffffb3bac5668000
RIP: 0010:_request_firmware+0x7fd/0x910
RSP: 0018:ffffb3bac566b978 EFLAGS: 00010246
RAX: 00000000fffffffe RBX: ffff919b9b2becc0 RCX: ffff919b9b2bece8
RDX: ffff919b81a99a80 RSI: 0000000000000206 RDI: 0000000000000000
RBP: ffffb3bac566b9e0 R08: ffff919b9f30f4b8 R09: 0000000000000002
R10: 00000120d2b47b48 R11: 0000000000000000 R12: ffffb3bac566ba18
R13: ffff919b823f4780 R14: ffff919b907c5000 R15: 0000000000003a98
FS:  00007fae4d803740(0000) GS:ffff919b9f300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000038 CR3: 00000008507b8000 CR4: 00000000001406e0
Call Trace:
request_firmware+0x37/0x50
...

Fixes: 5d47ec02c37e ("firmware: Correct handling of fw_state_wait() return value")
Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
---
v2:
 - improve commit message.
---
 drivers/base/firmware_class.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 4497d263209f..ce142e6b2c72 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -1020,7 +1020,7 @@ static int _request_firmware_load(struct firmware_priv *fw_priv,
 	}
 
 	retval = fw_state_wait_timeout(&buf->fw_st, timeout);
-	if (retval < 0) {
+	if (retval == -ETIMEDOUT || retval == -ERESTARTSYS) {
 		mutex_lock(&fw_lock);
 		fw_load_abort(fw_priv);
 		mutex_unlock(&fw_lock);
-- 
2.11.0