diff mbox series

Input: iqs7222 - propagate some error codes correctly

Message ID 20220412153954.GA15406@kili
State Accepted
Commit eba697b3c30320933aeb19b0606c2099fe880e51
Headers show
Series Input: iqs7222 - propagate some error codes correctly | expand

Commit Message

Dan Carpenter April 12, 2022, 3:39 p.m. UTC 
If fwnode_property_count_u32() returns a negative error code then,
because of type promotion, the "count > ARRAY_SIZE(pins)" condition
will be true.  The negative "count" is type promoted to a high unsigned
size_t value.

That means the "else if (count < 0)" condition will always be false and
we don't print that error message or propagate the error code from
fwnode_property_count_u32() as intended.

Fix this by re-ordering the checks so that we check for negative first.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/input/misc/iqs7222.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

Comments

Dmitry Torokhov April 17, 2022, 8:04 p.m. UTC | #1 
On Tue, Apr 12, 2022 at 06:39:54PM +0300, Dan Carpenter wrote:
> If fwnode_property_count_u32() returns a negative error code then,
> because of type promotion, the "count > ARRAY_SIZE(pins)" condition
> will be true.  The negative "count" is type promoted to a high unsigned
> size_t value.
> 
> That means the "else if (count < 0)" condition will always be false and
> we don't print that error message or propagate the error code from
> fwnode_property_count_u32() as intended.
> 
> Fix this by re-ordering the checks so that we check for negative first.
> 
> Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied, thank you.
Jeff LaBundy April 17, 2022, 8:10 p.m. UTC | #2 
Hi Dan,

On Sun, Apr 17, 2022 at 01:04:33PM -0700, Dmitry Torokhov wrote:
> On Tue, Apr 12, 2022 at 06:39:54PM +0300, Dan Carpenter wrote:
> > If fwnode_property_count_u32() returns a negative error code then,
> > because of type promotion, the "count > ARRAY_SIZE(pins)" condition
> > will be true.  The negative "count" is type promoted to a high unsigned
> > size_t value.
> > 
> > That means the "else if (count < 0)" condition will always be false and
> > we don't print that error message or propagate the error code from
> > fwnode_property_count_u32() as intended.
> > 
> > Fix this by re-ordering the checks so that we check for negative first.
> > 
> > Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

A bit late on my part, but for what it's worth:

Acked-by: Jeff LaBundy <jeff@labundy.com>

Thanks for sending this; it's a great catch.

> 
> Applied, thank you.
> 
> -- 
> Dmitry

Kind regards,
Jeff LaBundy
diff mbox series

Patch

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index d800f71043a5..c0b273222092 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -1677,14 +1677,14 @@  static int iqs7222_parse_cycle(struct iqs7222_private *iqs7222, int cycle_index)
 		return 0;
 
 	count = fwnode_property_count_u32(cycle_node, "azoteq,tx-enable");
-	if (count > ARRAY_SIZE(pins)) {
-		dev_err(&client->dev, "Invalid number of %s CTx pins\n",
-			fwnode_get_name(cycle_node));
-		return -EINVAL;
-	} else if (count < 0) {
+	if (count < 0) {
 		dev_err(&client->dev, "Failed to count %s CTx pins: %d\n",
 			fwnode_get_name(cycle_node), count);
 		return count;
+	} else if (count > ARRAY_SIZE(pins)) {
+		dev_err(&client->dev, "Invalid number of %s CTx pins\n",
+			fwnode_get_name(cycle_node));
+		return -EINVAL;
 	}
 
 	error = fwnode_property_read_u32_array(cycle_node, "azoteq,tx-enable",
@@ -1807,16 +1807,16 @@  static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 
 		count = fwnode_property_count_u32(chan_node,
 						  "azoteq,rx-enable");
-		if (count > ARRAY_SIZE(pins)) {
-			dev_err(&client->dev,
-				"Invalid number of %s CRx pins\n",
-				fwnode_get_name(chan_node));
-			return -EINVAL;
-		} else if (count < 0) {
+		if (count < 0) {
 			dev_err(&client->dev,
 				"Failed to count %s CRx pins: %d\n",
 				fwnode_get_name(chan_node), count);
 			return count;
+		} else if (count > ARRAY_SIZE(pins)) {
+			dev_err(&client->dev,
+				"Invalid number of %s CRx pins\n",
+				fwnode_get_name(chan_node));
+			return -EINVAL;
 		}
 
 		error = fwnode_property_read_u32_array(chan_node,
@@ -1975,14 +1975,14 @@  static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 	 * the specified resolution.
 	 */
 	count = fwnode_property_count_u32(sldr_node, "azoteq,channel-select");
-	if (count < 3 || count > ARRAY_SIZE(chan_sel)) {
-		dev_err(&client->dev, "Invalid number of %s channels\n",
-			fwnode_get_name(sldr_node));
-		return -EINVAL;
-	} else if (count < 0) {
+	if (count < 0) {
 		dev_err(&client->dev, "Failed to count %s channels: %d\n",
 			fwnode_get_name(sldr_node), count);
 		return count;
+	} else if (count < 3 || count > ARRAY_SIZE(chan_sel)) {
+		dev_err(&client->dev, "Invalid number of %s channels\n",
+			fwnode_get_name(sldr_node));
+		return -EINVAL;
 	}
 
 	error = fwnode_property_read_u32_array(sldr_node,