diff mbox series

xdp: remove WARN() from __xdp_reg_mem_model()

Message ID 20240617162708.492159-1-d.dulov@aladdin.ru (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series xdp: remove WARN() from __xdp_reg_mem_model() | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 845 this patch: 845
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 849 this patch: 849
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 852 this patch: 852
netdev/checkpatch warning WARNING: Possible repeated word: 'Google'
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-06-18--12-00 (tests: 654)

Commit Message

Daniil Dulov June 17, 2024, 4:27 p.m. UTC
Syzkaller reports a warning in __xdp_reg_mem_model().

The warning occurs only if __mem_id_init_hash_table() returns
an error. It returns the error in two cases:

    1. memory allocation fails;
    2. rhashtable_init() fails when some fields of rhashtable_params
       struct are not initialized properly.

The second case cannot happen since there is a static const
rhashtable_params struct with valid fields. So, warning is only triggered
when there is a problem with memory allocation.

Thus, there is no sense in using WARN() to handle this error and it can be
safely removed.

WARNING: CPU: 0 PID: 5065 at net/core/xdp.c:299 __xdp_reg_mem_model+0x2d9/0x650 net/core/xdp.c:299

CPU: 0 PID: 5065 Comm: syz-executor883 Not tainted 6.8.0-syzkaller-05271-gf99c5f563c17 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
RIP: 0010:__xdp_reg_mem_model+0x2d9/0x650 net/core/xdp.c:299

Call Trace:
 xdp_reg_mem_model+0x22/0x40 net/core/xdp.c:344
 xdp_test_run_setup net/bpf/test_run.c:188 [inline]
 bpf_test_run_xdp_live+0x365/0x1e90 net/bpf/test_run.c:377
 bpf_prog_test_run_xdp+0x813/0x11b0 net/bpf/test_run.c:1267
 bpf_prog_test_run+0x33a/0x3b0 kernel/bpf/syscall.c:4240
 __sys_bpf+0x48d/0x810 kernel/bpf/syscall.c:5649
 __do_sys_bpf kernel/bpf/syscall.c:5738 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:5736 [inline]
 __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5736
 do_syscall_64+0xfb/0x240
 entry_SYSCALL_64_after_hwframe+0x6d/0x75

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
---
 net/core/xdp.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Jesper Dangaard Brouer June 18, 2024, 9:13 a.m. UTC | #1
On 17/06/2024 18.27, Daniil Dulov wrote:
> Syzkaller reports a warning in __xdp_reg_mem_model().
> 
> The warning occurs only if __mem_id_init_hash_table() returns
> an error. It returns the error in two cases:
> 
>      1. memory allocation fails;
>      2. rhashtable_init() fails when some fields of rhashtable_params
>         struct are not initialized properly.
> 
> The second case cannot happen since there is a static const
> rhashtable_params struct with valid fields. So, warning is only triggered
> when there is a problem with memory allocation.
> 
> Thus, there is no sense in using WARN() to handle this error and it can be
> safely removed.
> 
> WARNING: CPU: 0 PID: 5065 at net/core/xdp.c:299 __xdp_reg_mem_model+0x2d9/0x650 net/core/xdp.c:299
> 
> CPU: 0 PID: 5065 Comm: syz-executor883 Not tainted 6.8.0-syzkaller-05271-gf99c5f563c17 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> RIP: 0010:__xdp_reg_mem_model+0x2d9/0x650 net/core/xdp.c:299
> 
> Call Trace:
>   xdp_reg_mem_model+0x22/0x40 net/core/xdp.c:344
>   xdp_test_run_setup net/bpf/test_run.c:188 [inline]
>   bpf_test_run_xdp_live+0x365/0x1e90 net/bpf/test_run.c:377
>   bpf_prog_test_run_xdp+0x813/0x11b0 net/bpf/test_run.c:1267
>   bpf_prog_test_run+0x33a/0x3b0 kernel/bpf/syscall.c:4240
>   __sys_bpf+0x48d/0x810 kernel/bpf/syscall.c:5649
>   __do_sys_bpf kernel/bpf/syscall.c:5738 [inline]
>   __se_sys_bpf kernel/bpf/syscall.c:5736 [inline]
>   __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5736
>   do_syscall_64+0xfb/0x240
>   entry_SYSCALL_64_after_hwframe+0x6d/0x75
> 
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> 
> Fixes: 8d5d88527587 ("xdp: rhashtable with allocator ID to pointer mapping")
> Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
> ---
>   net/core/xdp.c | 1 -
>   1 file changed, 1 deletion(-)
> 

Sure, looks like we can remove this WARN_ON()

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>

> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 41693154e426..fb2f00e3f701 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -296,7 +296,6 @@ static struct xdp_mem_allocator *__xdp_reg_mem_model(struct xdp_mem_info *mem,
>   		ret = __mem_id_init_hash_table();
>   		mutex_unlock(&mem_id_lock);
>   		if (ret < 0) {
> -			WARN_ON(1);
>   			return ERR_PTR(ret);
>   		}
>   	}
Jakub Kicinski June 19, 2024, 1:23 a.m. UTC | #2
On Mon, 17 Jun 2024 19:27:08 +0300 Daniil Dulov wrote:
>  		if (ret < 0) {
> -			WARN_ON(1);
>  			return ERR_PTR(ret);
>  		}

nit: the brackets are no longer necessary, then
diff mbox series

Patch

diff --git a/net/core/xdp.c b/net/core/xdp.c
index 41693154e426..fb2f00e3f701 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -296,7 +296,6 @@  static struct xdp_mem_allocator *__xdp_reg_mem_model(struct xdp_mem_info *mem,
 		ret = __mem_id_init_hash_table();
 		mutex_unlock(&mem_id_lock);
 		if (ret < 0) {
-			WARN_ON(1);
 			return ERR_PTR(ret);
 		}
 	}