netdev - Re: [PATCH] ethtool: fix possible NULL dereference in main() via argp

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

Message-ID: <ddc6kqji5zcv2tw6ssmbdbhgeuffjwayj224rb2de22yfz6ycr@wgqkwoxqeohy>
Date: Wed, 11 Jun 2025 01:04:58 +0200
From: Michal Kubecek <mkubecek@...e.cz>
To: ant.v.moryakov@...il.com
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH] ethtool: fix possible NULL dereference in main() via argp

Dne Sun, May 18, 2025 at 04:13:32PM GMT, ant.v.moryakov@...il.com napsal:
> From: AntonMoryakov <ant.v.moryakov@...il.com>
> 
> Static analyzer (Svace) reported a possible null pointer dereference
> in main(), where the pointer 'argp' is dereferenced without checking
> whether it is NULL. Specifically, if 'argc' is 0 or reduced to 0 by
> parsing global options, then '*argp' would cause undefined behavior.
> 
> This patch adds a NULL check for 'argp' before calling find_option().
> 
> This resolves:
> DEREF_AFTER_NULL.EX.COND: ethtool.c:6391
> 
> Found by Svace static analysis tool.
> 
> Signed-off-by: Anton Moryakov <ant.v.moryakov@...il.com>
> 
> ---
>  ethtool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ethtool.c b/ethtool.c
> index 327a2da..4601051 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -6634,7 +6634,7 @@ int main(int argc, char **argp)
>  	 * name to get settings for (which we don't expect to begin
>  	 * with '-').
>  	 */
> -	if (!*argp)
> +	if (!argp || !*argp)
>  		exit_bad_args();
>  
>  	k = find_option(*argp);

This doesn't seem right. First, the ISO C standard guarantees that
argv[argc] is a null pointer and if argc > 0, then the array members up
to argv[argc - 1] are pointers to strings. This IMHO implies that the
value of argp here cannot really be null.

Second, even if we wanted to be protected against such incorrect
implementation, there would be no point doing it here, after we already
worked with argp through all the block processing global options. It
would IMHO make much more sense to check it right at the beginning.

Michal

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)