[PATCH] ethtool: fix potential NULL pointer dereference in find_option

[PATCH] ethtool: fix potential NULL pointer dereference in find_option

[Anton Moryakov]

Static analyzer reported a possible NULL pointer dereference:

- In main(), 'argp' is dereferenced and passed to find_option()
	without checking if *argp is NULL.
	
- The function find_option() does not validate its input argument,
	which may lead to undefined behavior when using strncmp() or strcspn()
	with a NULL pointer.

This patch adds proper NULL check in find_option() to prevent invalid memory access.
Additionally, it improves robustness by making sure that the input argument
is valid before passing it to find_option().

Found by Svace static analysis tool.

Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
---
 ethtool.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index 0513a1a..4250add 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6395,6 +6395,9 @@ static int show_usage(struct cmd_context *ctx __maybe_unused)
 
 static int find_option(char *arg)
 {
+	if(!arg)
+		return -1;
+		
 	const char *opt;
 	size_t len;
 	int k;
-- 
2.39.2

Re: [PATCH] ethtool: fix potential NULL pointer dereference in find_option

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

On Wed, Jul 09, 2025 at 01:41:20PM GMT, Anton Moryakov wrote:
> Static analyzer reported a possible NULL pointer dereference:
> 
> - In main(), 'argp' is dereferenced and passed to find_option()
> 	without checking if *argp is NULL.

Not true. This is what the code looks like:

	if (!*argp)
		exit_bad_args();

	k = find_option(*argp); 

On the other hand, there is no such check when find_option() is called
from do_perqueue(). But if we really want to address this, I would
rather do it in a consistent way, i.e. the same way in both places.

However, I'm still not convinced the inconsistency between argc and
argp[] you are trying to prevent can actually happen on Linux. AFAIK all
exec* functions are only library wrappers for execve() which is passed
only the argv[] array and the argc count is determined from it by
finding the first null pointer. Unless I'm missing something important,
this means that argp[i] can never be null for i < argc.

Do you have a scenario (not assuming a serious kernel bug) that would
result in null pointer in argp[] before the argc index?

> Additionally, it improves robustness by making sure that the input argument
> is valid before passing it to find_option().

The patch does not actually do this, it only changes the code inside
find_option().

> 
> Found by Svace static analysis tool.
> 
> Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
> ---
>  ethtool.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ethtool.c b/ethtool.c
> index 0513a1a..4250add 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -6395,6 +6395,9 @@ static int show_usage(struct cmd_context *ctx __maybe_unused)
>  
>  static int find_option(char *arg)
>  {
> +	if(!arg)

Missing space between "if" and "(".

> +		return -1;
> +		

You add whitespace on this empty line.


Michal

>  	const char *opt;
>  	size_t len;
>  	int k;
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]