netdev - [PATCH ethtool v2 06/13] ethtool: fix uninitialized local variable use

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

Message-Id: <20221208011122.2343363-7-jesse.brandeburg@intel.com>
Date:   Wed,  7 Dec 2022 17:11:15 -0800
From:   Jesse Brandeburg <jesse.brandeburg@...el.com>
To:     mkubecek@...e.cz
Cc:     netdev@...r.kernel.org,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Andrew Lunn <andrew@...n.ch>
Subject: [PATCH ethtool v2 06/13] ethtool: fix uninitialized local variable use

'$ scan-build make' reports:

netlink/parser.c:252:25: warning: The left operand of '*' is a garbage value [core.UndefinedBinaryOperatorResult]
        cm = (uint32_t)(meters * 100 + 0.5);
                        ~~~~~~ ^

This is a little more complicated than it seems, but for some
unexplained reason, parse_float always returns integers but was declared
to return a float. This is confusing at best. In the case of the error
above, parse_float could conceivably return without initializing it's
output variable, and because the function return variable was declared
as float but downgraded to an int via assignment (-Wconversion anyone?)
upon the return, the return value could be ignored.

To fix the bug above, declare an initial value for meters, and make sure
that parse_float() always returns an integer value.

It would probably be even more ideal if parse_float always initialized
it's output variables before even checking for input errors, but that's
for some future day.

CC: Andrew Lunn <andrew@...n.ch>
Fixes: 9561db9b76f4 ("Add cable test TDR support")
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
---
 netlink/parser.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/netlink/parser.c b/netlink/parser.c
index f982f229a040..70f451008eb4 100644
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -54,8 +54,7 @@ static bool __prefix_0x(const char *p)
 	return p[0] == '0' && (p[1] == 'x' || p[1] == 'X');
 }

-static float parse_float(const char *arg, float *result, float min,
-			 float max)
+static int parse_float(const char *arg, float *result, float min, float max)
 {
 	char *endptr;
 	float val;
@@ -237,7 +236,7 @@ int nl_parse_direct_m2cm(struct nl_context *nlctx, uint16_t type,
 			 struct nl_msg_buff *msgbuff, void *dest)
 {
 	const char *arg = *nlctx->argp;
-	float meters;
+	float meters = 0;
 	uint32_t cm;
 	int ret;

-- 
2.31.1