linux-kernel - Re: [kees:devel/overflow/sanitizers] [overflow] 660787b56e: UBSAN:signed-integer-overflow_in_lib/test_memcat

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

Date: Tue, 30 Jan 2024 16:23:06 -0800
From: Kees Cook <keescook@...omium.org>
To: kernel test robot <oliver.sang@...el.com>
Cc: oe-lkp@...ts.linux.dev, lkp@...el.com,
	Justin Stitt <justinstitt@...gle.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Nathan Chancellor <nathan@...nel.org>,
	Nick Desaulniers <ndesaulniers@...gle.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Marco Elver <elver@...gle.com>, Hao Luo <haoluo@...gle.com>,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>,
	workflows@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-kbuild@...r.kernel.org
Subject: Re: [kees:devel/overflow/sanitizers] [overflow] 660787b56e:
 UBSAN:signed-integer-overflow_in_lib/test_memcat_p.c

On Tue, Jan 30, 2024 at 10:52:56PM +0800, kernel test robot wrote:
> 
> 
> Hello,
> 
> kernel test robot noticed "UBSAN:signed-integer-overflow_in_lib/test_memcat_p.c" on:
> 
> commit: 660787b56e6e97ddc34c7882cbe1228f4040ef74 ("overflow: Reintroduce signed and unsigned overflow sanitizers")
> https://git.kernel.org/cgit/linux/kernel/git/kees/linux.git devel/overflow/sanitizers
> 
> in testcase: boot
> 
> compiler: gcc-11
> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
> 
> (please refer to attached dmesg/kmsg for entire log/backtrace)
> 
> 
> we noticed this commit is reintroducing "signed and unsigned overflow
> sanitizers", there is below config diff between parent and this commit in our
> buildings:
> 
> --- ea804316c9db5148d2bb0c1f40f70d7a83404638/.config    2024-01-26 22:09:35.046768122 +0800
> +++ 660787b56e6e97ddc34c7882cbe1228f4040ef74/.config    2024-01-26 19:53:20.693434428 +0800
> @@ -6706,6 +6706,7 @@ CONFIG_UBSAN_BOUNDS_STRICT=y
>  CONFIG_UBSAN_SHIFT=y
>  # CONFIG_UBSAN_DIV_ZERO is not set
>  CONFIG_UBSAN_UNREACHABLE=y
> +CONFIG_UBSAN_SIGNED_WRAP=y
>  # CONFIG_UBSAN_BOOL is not set
>  # CONFIG_UBSAN_ENUM is not set
>  # CONFIG_UBSAN_ALIGNMENT is not set

Hi! Thanks for the testing!

The added Kconfig is:

+config UBSAN_SIGNED_WRAP
+       bool "Perform checking for signed arithmetic wrap-around"
+       default UBSAN
+       depends on !COMPILE_TEST
+       depends on $(cc-option,-fsanitize=signed-integer-overflow)

So it looks like since your build already had CONFIG_UBSAN=y the signed
sanitizer got enabled too since it doesn't set CONFIG_COMPILE_TEST.

> while testing, we observed below different (and same part) between parent and
> this commit:
> 
> ea804316c9db5148 660787b56e6e97ddc34c7882cbe
> ---------------- ---------------------------
>        fail:runs  %reproduction    fail:runs
>            |             |             |
>           6:6            0%           6:6     dmesg.UBSAN:shift-out-of-bounds_in_arch/x86/kernel/cpu/intel.c
>           6:6            0%           6:6     dmesg.UBSAN:shift-out-of-bounds_in_arch/x86/kernel/cpu/topology.c
>           6:6            0%           6:6     dmesg.UBSAN:shift-out-of-bounds_in_fs/namespace.c
>           6:6            0%           6:6     dmesg.UBSAN:shift-out-of-bounds_in_fs/read_write.c
>           6:6            0%           6:6     dmesg.UBSAN:shift-out-of-bounds_in_include/linux/rhashtable.h
>           6:6            0%           6:6     dmesg.UBSAN:shift-out-of-bounds_in_include/net/tcp.h

Are these shift-out-of-bounds warnings new?

>            :6          100%           6:6     dmesg.UBSAN:signed-integer-overflow_in_lib/test_memcat_p.c

This is new for sure, catching an issue you show below...

> this looks like the commit uncovered issue. but since it's hard for us to back
> port this commit to each commit while bisecting, we cannot capture the real
> first bad commit. not sure if this report could help somebody to investigate
> the real issue?

Yeah, I think there is an unexpected wrap-around in test_memcat_p.c:

> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@...el.com>
> | Closes: https://lore.kernel.org/oe-lkp/202401302219.db90a6d5-oliver.sang@intel.com
> 
> 
> [   42.894536][    T1] ------------[ cut here ]------------
> [   42.895474][    T1] UBSAN: signed-integer-overflow in lib/test_memcat_p.c:47:10
> [   42.897128][    T1] 6570 * 725861 cannot be represented in type 'int'

I'm surprised to see the sanitizer catching anything here since the
kernel is built with -fno-strict-overflow, but regardless, I'll send a
patch...

-Kees

-- 
Kees Cook