diff mbox series

[net] llc: fix out-of-bound array index in llc_sk_dev_hash()

Message ID 20211105214214.2259841-1-eric.dumazet@gmail.com (mailing list archive)
State Accepted
Commit 8ac9dfd58b138f7e82098a4e0a0d46858b12215b
Delegated to: Netdev Maintainers
Headers show
Series [net] llc: fix out-of-bound array index in llc_sk_dev_hash() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 12 this patch: 12
netdev/cc_maintainers fail 1 blamed authors not CCed: opurdila@ixiacom.com; 1 maintainers not CCed: opurdila@ixiacom.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 12 this patch: 12
netdev/checkpatch warning WARNING: Possible repeated word: 'Google'
netdev/kdoc success Errors and warnings before: 12 this patch: 12
netdev/source_inline success Was 0 now: 0

Commit Message

Eric Dumazet Nov. 5, 2021, 9:42 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

Both ifindex and LLC_SK_DEV_HASH_ENTRIES are signed.

This means that (ifindex % LLC_SK_DEV_HASH_ENTRIES) is negative
if @ifindex is negative.

We could simply make LLC_SK_DEV_HASH_ENTRIES unsigned.

In this patch I chose to use hash_32() to get more entropy
from @ifindex, like llc_sk_laddr_hashfn().

UBSAN: array-index-out-of-bounds in ./include/net/llc.h:75:26
index -43 is out of range for type 'hlist_head [64]'
CPU: 1 PID: 20999 Comm: syz-executor.3 Not tainted 5.15.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
 ubsan_epilogue+0xb/0x5a lib/ubsan.c:151
 __ubsan_handle_out_of_bounds.cold+0x62/0x6c lib/ubsan.c:291
 llc_sk_dev_hash include/net/llc.h:75 [inline]
 llc_sap_add_socket+0x49c/0x520 net/llc/llc_conn.c:697
 llc_ui_bind+0x680/0xd70 net/llc/af_llc.c:404
 __sys_bind+0x1e9/0x250 net/socket.c:1693
 __do_sys_bind net/socket.c:1704 [inline]
 __se_sys_bind net/socket.c:1702 [inline]
 __x64_sys_bind+0x6f/0xb0 net/socket.c:1702
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7fa503407ae9

Fixes: 6d2e3ea28446 ("llc: use a device based hash table to speed up multicast delivery")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
 include/net/llc.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

patchwork-bot+netdevbpf@kernel.org Nov. 7, 2021, 7:30 p.m. UTC | #1 
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri,  5 Nov 2021 14:42:14 -0700 you wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Both ifindex and LLC_SK_DEV_HASH_ENTRIES are signed.
> 
> This means that (ifindex % LLC_SK_DEV_HASH_ENTRIES) is negative
> if @ifindex is negative.
> 
> [...]

Here is the summary with links:
  - [net] llc: fix out-of-bound array index in llc_sk_dev_hash()
    https://git.kernel.org/netdev/net/c/8ac9dfd58b13

You are awesome, thank you!
diff mbox series

Patch

diff --git a/include/net/llc.h b/include/net/llc.h
index fd1f9a3fd8dda463cc24d95e0d3a528e505927b4..e250dca03963bf14750d16ebf1cb6d976b7206d3 100644
--- a/include/net/llc.h
+++ b/include/net/llc.h
@@ -72,7 +72,9 @@  struct llc_sap {
 static inline
 struct hlist_head *llc_sk_dev_hash(struct llc_sap *sap, int ifindex)
 {
-	return &sap->sk_dev_hash[ifindex % LLC_SK_DEV_HASH_ENTRIES];
+	u32 bucket = hash_32(ifindex, LLC_SK_DEV_HASH_BITS);
+
+	return &sap->sk_dev_hash[bucket];
 }
 
 static inline