netdev - [PATCH ethtool v2] Fix argc and argp handling issues

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: <4b89caeddf355b07da0ba68ea058a94e5a55ff59.1683549760.git.nvinson234@gmail.com>
Date: Mon,  8 May 2023 16:23:07 -0400
From: Nicholas Vinson <nvinson234@...il.com>
To: mkubeck@...e.cz
Cc: Nicholas Vinson <nvinson234@...il.com>,
	netdev@...r.kernel.org
Subject: [PATCH ethtool v2] Fix argc and argp handling issues

Fixes issues that were originally found using gcc's static analyzer. The
flags used to invoke the analyzer are given below.

Upon manual review of the results and discussion of the previous patch
'[PATCH ethtool 3/3] Fix potential null-pointer deference issues.', it
was determined that when using a kernel lacking the execve patch ( see
https://github.com/gregkh/linux/commit/dcd46d897adb70d63e025f175a00a89797d31a43),
it is possible for argc to be 0 and argp to be an array with only a
single NULL entry. This scenario would cause ethtool to read beyond the
bounds of the argp array. However, this scenario should not be possible
for any Linux kernel released within the last two years should have the
execve patch applied.

    CFLAGS=-march=native -O2 -pipe -fanalyzer       \
        -Werror=analyzer-va-arg-type-mismatch       \
        -Werror=analyzer-va-list-exhausted          \
        -Werror=analyzer-va-list-leak               \
        -Werror=analyzer-va-list-use-after-va-end

    CXXCFLAGS=-march=native -O2                     \
        -pipe -fanalyzer                            \
        -Werror=analyzer-va-arg-type-mismatch       \
        -Werror=analyzer-va-list-exhausted          \
        -Werror=analyzer-va-list-leak               \
        -Werror=analyzer-va-list-use-after-va-end

    LDFLAGS="-Wl,-O1 -Wl,--as-needed"

    GCC version is gcc (Gentoo 13.1.0-r1 p1) 13.1.0

    Additional Information:
    https://patchwork.kernel.org/project/netdevbpf/patch/20221208011122.2343363-8-jesse.brandeburg@intel.com/

    Signed-off-by: Nicholas Vinson <nvinson234@...il.com>
    Reviewed-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
---
 ethtool.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ethtool.c b/ethtool.c
index 98690df..0752fe4 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6405,6 +6405,9 @@ int main(int argc, char **argp)
 
 	init_global_link_mode_masks();
 
+	if (argc < 2)
+		exit_bad_args();
+
 	/* Skip command name */
 	argp++;
 	argc--;
@@ -6449,7 +6452,7 @@ int main(int argc, char **argp)
 	 * name to get settings for (which we don't expect to begin
 	 * with '-').
 	 */
-	if (argc == 0)
+	if (!*argp)
 		exit_bad_args();
 
 	k = find_option(*argp);
-- 
2.40.1