linux-kernel - [PATCH RFC] fpga: socfpga: handle interrupted case as non-success

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

Message-Id: <1494059621-8235-1-git-send-email-der.herr@hofr.at>
Date:   Sat,  6 May 2017 10:33:41 +0200
From:   Nicholas Mc Guire <der.herr@...r.at>
To:     Alan Tull <atull@...nel.org>
Cc:     Moritz Fischer <moritz.fischer@...us.com>,
        linux-fpga@...r.kernel.org, linux-kernel@...r.kernel.org,
        Nicholas Mc Guire <der.herr@...r.at>
Subject: [PATCH RFC] fpga: socfpga: handle interrupted case as non-success

 wait_for_completion_interruptible_timeout() can return 0 (timeout)
or -ERESTARTSYS when interrupted - the current code treats the
interrupted case as success which is wrong and could lead to hard
to reproduce errors. Fix this by returning non-0 in both cases
to the calling side. This also changes the timeout variable to the
proper type as wait_for_completion_interruptible_timeout() returns
long not int.

Fixes: commit fab6266e82a8 ("fpga manager: add driver for socfpga fpga manager")
Signed-off-by: Nicholas Mc Guire <der.herr@...r.at>
---

It is not really clear to me why the _interruptible_ version of 
wait_for_completion is in use here - so maybe the proper mitigation
is to simply change the call to wait_for_completion_timeout() and
drop the interrupted case all together as the timeout of 10ms is 
not that long that waiting for timeout would be problematic. Someone
that knows the intent of selecting the _interruptible_ call would
need to check if this is a more reasonable option.

Patch was compile tested with:

Patch is against v4.11 (localversion-next is next-20170505)

 drivers/fpga/socfpga.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/fpga/socfpga.c b/drivers/fpga/socfpga.c
index b6672e6..532abd6 100644
--- a/drivers/fpga/socfpga.c
+++ b/drivers/fpga/socfpga.c
@@ -312,7 +312,8 @@ static irqreturn_t socfpga_fpga_isr(int irq, void *dev_id)

 static int socfpga_fpga_wait_for_config_done(struct socfpga_fpga_priv *priv)
 {
-	int timeout, ret = 0;
+	ret = 0;
+	long timeout;

 	socfpga_fpga_disable_irqs(priv);
 	init_completion(&priv->status_complete);
@@ -323,6 +324,8 @@ static int socfpga_fpga_wait_for_config_done(struct socfpga_fpga_priv *priv)
 						msecs_to_jiffies(10));
 	if (timeout == 0)
 		ret = -ETIMEDOUT;
+	else if (timeout == -ERESTARTSYS)
+		ret = -ERESTARTSYS;

 	socfpga_fpga_disable_irqs(priv);
 	return ret;
-- 
2.1.4