netdev - Re: [PATCH linux-next] tools: fix warning comparing pointer to 0

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: <491f06b5-3680-012a-f1d0-9831aa18e56a@iogearbox.net>
Date:   Mon, 23 Aug 2021 17:54:07 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     CGEL <cgel.zte@...il.com>, Shuah Khan <shuah@...nel.org>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Andrei Matei <andreimatei1@...il.com>,
        linux-kselftest@...r.kernel.org, netdev@...r.kernel.org,
        bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
        jing yangyang <jing.yangyang@....com.cn>,
        Zeal Robot <zealci@....com.cn>
Subject: Re: [PATCH linux-next] tools: fix warning comparing pointer to 0

On 8/20/21 5:30 AM, CGEL wrote:
> From: jing yangyang <jing.yangyang@....com.cn>
> 
> Fix the following coccicheck warning:
> ./tools/testing/selftests/bpf/progs/profiler.inc.h:364:18-22:WARNING
> comparing pointer to 0
> ./tools/testing/selftests/bpf/progs/profiler.inc.h:537:23-27:WARNING
> comparing pointer to 0
> ./tools/testing/selftests/bpf/progs/profiler.inc.h:544:21-25:WARNING
> comparing pointer to 0
> ./tools/testing/selftests/bpf/progs/profiler.inc.h:770:13-17:WARNING
> comparing pointer to 0
> 
> Reported-by: Zeal Robot <zealci@....com.cn>
> Signed-off-by: jing yangyang <jing.yangyang@....com.cn>

Please properly explain in the commit message what this 'fixes' exactly and
why it is needed.

> ---
>   tools/testing/selftests/bpf/progs/profiler.inc.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h
> index 4896fdf..5c0bdab 100644
> --- a/tools/testing/selftests/bpf/progs/profiler.inc.h
> +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
> @@ -361,7 +361,7 @@ static INLINE void* populate_var_metadata(struct var_metadata_t* metadata,
>   	int zero = 0;
>   	struct var_kill_data_t* kill_data = bpf_map_lookup_elem(&data_heap, &zero);
>   
> -	if (kill_data == NULL)
> +	if (!kill_dat)

And please don't send broken stuff like this.

>   		return NULL;
>   	struct task_struct* task = (struct task_struct*)bpf_get_current_task();
>   
> @@ -534,14 +534,14 @@ static INLINE bool is_dentry_allowed_for_filemod(struct dentry* file_dentry,
>   	*device_id = dev_id;
>   	bool* allowed_device = bpf_map_lookup_elem(&allowed_devices, &dev_id);
>   
> -	if (allowed_device == NULL)
> +	if (!allowed_device)
>   		return false;
>   
>   	u64 ino = BPF_CORE_READ(file_dentry, d_inode, i_ino);
>   	*file_ino = ino;
>   	bool* allowed_file = bpf_map_lookup_elem(&allowed_file_inodes, &ino);
>   
> -	if (allowed_file == NULL)
> +	if (!allowed_fil)

... same. You did not bother to compile test even.

>   		if (!is_ancestor_in_allowed_inodes(BPF_CORE_READ(file_dentry, d_parent)))
>   			return false;
>   	return true;
> @@ -689,7 +689,7 @@ int raw_tracepoint__sched_process_exec(struct bpf_raw_tracepoint_args* ctx)
>   	u64 inode = BPF_CORE_READ(bprm, file, f_inode, i_ino);
>   
>   	bool* should_filter_binprm = bpf_map_lookup_elem(&disallowed_exec_inodes, &inode);
> -	if (should_filter_binprm != NULL)
> +	if (should_filter_binprm)
>   		goto out;
>   
>   	int zero = 0;
>