linux-kernel - [PATCH] usb: typec: tipd: improve handling of failures in interrupt handlers

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20220409130013.1474412-1-trix@redhat.com>
Date:   Sat,  9 Apr 2022 09:00:13 -0400
From:   Tom Rix <trix@...hat.com>
To:     heikki.krogerus@...ux.intel.com, gregkh@...uxfoundation.org,
        nathan@...nel.org, ndesaulniers@...gle.com, sven@...npeter.dev,
        alyssa@...enzweig.io, marcan@...can.st, martink@...teo.de,
        saranya.gopal@...el.com
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev, Tom Rix <trix@...hat.com>
Subject: [PATCH] usb: typec: tipd: improve handling of failures in interrupt handlers

clang static analysis reports this representative issue
core.c:516:6: warning: Branch condition evaluates
  to a garbage value
  if (event)
      ^~~~~

In cd321x_interrupt(), a successful call to
tps6598x_read64() is the only way event is set,
and if a failure happens the irq should not be
reported as handled.

Instead of initializing event, rework the
usage of ret by initializing it to IRQ_NONE
and then setting it when event is known to
be not zero.  This removes the if-statement
before the return.

tps6598x_interrupt() is similar.

Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
Signed-off-by: Tom Rix <trix@...hat.com>
---
 drivers/usb/typec/tipd/core.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 16b4560216ba..88a20cc15da4 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -478,12 +478,11 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
 	struct tps6598x *tps = data;
 	u64 event;
 	u32 status;
-	int ret;
+	int ret = IRQ_NONE;
 
 	mutex_lock(&tps->lock);
 
-	ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event);
-	if (ret) {
+	if (tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event)) {
 		dev_err(tps->dev, "%s: failed to read events\n", __func__);
 		goto err_unlock;
 	}
@@ -492,6 +491,8 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
 	if (!event)
 		goto err_unlock;
 
+	ret = IRQ_HANDLED;
+
 	if (!tps6598x_read_status(tps, &status))
 		goto err_clear_ints;
 
@@ -513,9 +514,7 @@ static irqreturn_t cd321x_interrupt(int irq, void *data)
 err_unlock:
 	mutex_unlock(&tps->lock);
 
-	if (event)
-		return IRQ_HANDLED;
-	return IRQ_NONE;
+	return ret;
 }
 
 static irqreturn_t tps6598x_interrupt(int irq, void *data)
@@ -524,13 +523,12 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 	u64 event1;
 	u64 event2;
 	u32 status;
-	int ret;
+	int ret = IRQ_NONE;
 
 	mutex_lock(&tps->lock);
 
-	ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event1);
-	ret |= tps6598x_read64(tps, TPS_REG_INT_EVENT2, &event2);
-	if (ret) {
+	if (tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event1) ||
+	    tps6598x_read64(tps, TPS_REG_INT_EVENT2, &event2)) {
 		dev_err(tps->dev, "%s: failed to read events\n", __func__);
 		goto err_unlock;
 	}
@@ -539,6 +537,8 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 	if (!(event1 | event2))
 		goto err_unlock;
 
+	ret = IRQ_HANDLED;
+
 	if (!tps6598x_read_status(tps, &status))
 		goto err_clear_ints;
 
@@ -561,9 +561,7 @@ static irqreturn_t tps6598x_interrupt(int irq, void *data)
 err_unlock:
 	mutex_unlock(&tps->lock);
 
-	if (event1 | event2)
-		return IRQ_HANDLED;
-	return IRQ_NONE;
+	return ret;
 }
 
 static int tps6598x_check_mode(struct tps6598x *tps)
-- 
2.27.0