Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1283153 > unrolled thread

[PATCH v4 0/3] UBSAN: run-time undefined behavior sanity checker

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/3] UBSAN: run-time undefined behavior sanity checker Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-12-03 17:00 +0100
    [PATCH v4 2/3] mac80211: Prevent build failure with CONFIG_UBSAN=y Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-12-03 17:00 +0100
      Re: [PATCH v4 2/3] mac80211: Prevent build failure with  CONFIG_UBSAN=y Johannes Berg <johannes@sipsolutions.net> - 2015-12-03 18:10 +0100
        Re: [PATCH v4 2/3] mac80211: Prevent build failure with CONFIG_UBSAN=y Andrey Ryabinin <ryabinin.a.a@gmail.com> - 2015-12-03 20:20 +0100
    Re: [PATCH v4 0/3] UBSAN: run-time undefined behavior sanity  checker Andrew Morton <akpm@linux-foundation.org> - 2015-12-05 01:40 +0100
      Re: [PATCH v4 0/3] UBSAN: run-time undefined behavior sanity checker Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-12-07 17:50 +0100
      Re: [PATCH v4 0/3] UBSAN: run-time undefined behavior sanity checker Andrey Ryabinin <aryabinin@virtuozzo.com> - 2015-12-10 16:50 +0100

#1283153 — [PATCH v4 0/3] UBSAN: run-time undefined behavior sanity checker

UBSAN is run-time undefined behaviour checker. It uses compile-time
instrumentation to catch undefined behavior (UB). Compiler inserts code
that perform certain kinds of checks before operations that could cause UB.
If check fails (i.e. UB detected) __ubsan_handle_* function called to print error message.


Changes since v3:
   - Fixed build failure/warnings reported by kbuild robot.
   - Fixed typo per Sasha.

Changes since V2:
   - Dropped -fsanitize=nonnull-attribute. It checks whether null values
     are not passed to arguments marked as requiring a non-null value by
     the "nonnull" function attribute.

     We don't have much functions with such attribute (early_shadow_write() in arch/blackfin
     and GCC builtin functions: memcpy, memset, memmove, etc). Some kernel code deliberately
     passes NULL-ptr with 0-length to mem*(). This should be fine since we compile kernel
     with -fno-delete-null-pointer-checks. And NULL-ptr with != 0 length will just crash.
     So this options is useless in kernel since it produces only false positives.
     See also: http://thread.gmane.org/gmane.linux.kernel/1810656
       

   - Also dropped enabling/disabling various checkers via boot cmdline.
     Boot time flag only disable reports, it can't disable compile-time code instrumentation.
     Thus, if we ever will need to disable some checker it would be better to
     do it in compile time via Kconfig option.

   - Alignment checks produce too much noise if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set.
      Since there is no boottime option to disable alignment checks, CONFIG_UBSAN_ALIGNMENT
      was added. It's off by default if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set.
   
   - Couple other small misc changes/fixes.



Changes since v1:
   - Refactoring and cleanups in lib/ubsan.c including Sasha's complains.
   - Some spelling fixes from Randy
   - Fixed possible memory corruption on 64 big endian machines, spotted by Rasmus.
   - Links to the relevant GCC documentation added into changelog (Peter).
   - Added documentation.
   - Fix deadlock caused by kernel/printk/printk.c instrumentation
        (patch "kernel: printk: specify alignment for struct printk_log").
   - Dropped useless 'Indirect call of a function through a function pointer of the wrong type'
     checker. GCC doesn't support this, and as clang manual says it's for C++ only.
   - Added checker for __builtin_unreachable() calls.
   - Removed redundant -fno-sanitize=float-cast-overflow from CFLAGS.
   - Added lock to prevent mixing reports.


Andrey Ryabinin (3):
  kernel: printk: specify alignment for struct printk_log
  mac80211: Prevent build failure with CONFIG_UBSAN=y
  UBSAN: run-time undefined behavior sanity checker

 Documentation/ubsan.txt               |  84 +++++++
 Makefile                              |   3 +-
 arch/x86/Kconfig                      |   1 +
 arch/x86/boot/Makefile                |   1 +
 arch/x86/boot/compressed/Makefile     |   1 +
 arch/x86/entry/vdso/Makefile          |   1 +
 arch/x86/realmode/rm/Makefile         |   1 +
 drivers/firmware/efi/libstub/Makefile |   1 +
 include/linux/sched.h                 |   3 +
 kernel/printk/printk.c                |  10 +-
 lib/Kconfig.debug                     |   1 +
 lib/Kconfig.ubsan                     |  29 +++
 lib/Makefile                          |   3 +
 lib/ubsan.c                           | 456 ++++++++++++++++++++++++++++++++++
 lib/ubsan.h                           |  84 +++++++
 mm/kasan/Makefile                     |   1 +
 net/mac80211/debugfs.c                |   7 +-
 scripts/Makefile.lib                  |   6 +
 scripts/Makefile.ubsan                |  18 ++
 19 files changed, 700 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/ubsan.txt
 create mode 100644 lib/Kconfig.ubsan
 create mode 100644 lib/ubsan.c
 create mode 100644 lib/ubsan.h
 create mode 100644 scripts/Makefile.ubsan

-- 
2.4.10

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1283155 — [PATCH v4 2/3] mac80211: Prevent build failure with CONFIG_UBSAN=y

With upcoming CONFIG_UBSAN the following BUILD_BUG_ON in
net/mac80211/debugfs.c starts to trigger:
	BUILD_BUG_ON(hw_flag_names[NUM_IEEE80211_HW_FLAGS] != (void *)0x1);

It seems, that compiler instrumentation causes some code deoptimizations.
Because of that GCC is not being able to resolve condition in BUILD_BUG_ON()
at compile time.

We could make size of hw_flag_names array unspecified and replace the
condition in BUILD_BUG_ON() with following:
	ARRAY_SIZE(hw_flag_names) != NUM_IEEE80211_HW_FLAGS

That will have the same effect as before (adding new flag without updating
array will trigger build failure) except it doesn't fail with CONFIG_UBSAN.
As a bonus this patch slightly decreases size of hw_flag_names array.

Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
---
 net/mac80211/debugfs.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index abbdff0..3e24d0d 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -91,7 +91,7 @@ static const struct file_operations reset_ops = {
 };
 #endif
 
-static const char *hw_flag_names[NUM_IEEE80211_HW_FLAGS + 1] = {
+static const char *hw_flag_names[] = {
 #define FLAG(F)	[IEEE80211_HW_##F] = #F
 	FLAG(HAS_RATE_CONTROL),
 	FLAG(RX_INCLUDES_FCS),
@@ -126,9 +126,6 @@ static const char *hw_flag_names[NUM_IEEE80211_HW_FLAGS + 1] = {
 	FLAG(SUPPORTS_AMSDU_IN_AMPDU),
 	FLAG(BEACON_TX_STATUS),
 	FLAG(NEEDS_UNIQUE_STA_ADDR),
-
-	/* keep last for the build bug below */
-	(void *)0x1
 #undef FLAG
 };
 
@@ -148,7 +145,7 @@ static ssize_t hwflags_read(struct file *file, char __user *user_buf,
 	/* fail compilation if somebody adds or removes
 	 * a flag without updating the name array above
 	 */
-	BUILD_BUG_ON(hw_flag_names[NUM_IEEE80211_HW_FLAGS] != (void *)0x1);
+	BUILD_BUG_ON(ARRAY_SIZE(hw_flag_names) != NUM_IEEE80211_HW_FLAGS);
 
 	for (i = 0; i < NUM_IEEE80211_HW_FLAGS; i++) {
 		if (test_bit(i, local->hw.flags))
-- 
2.4.10

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283198 — Re: [PATCH v4 2/3] mac80211: Prevent build failure with CONFIG_UBSAN=y

On Thu, 2015-12-03 at 18:50 +0300, Andrey Ryabinin wrote:
> With upcoming CONFIG_UBSAN the following BUILD_BUG_ON in
> net/mac80211/debugfs.c starts to trigger:
> 	BUILD_BUG_ON(hw_flag_names[NUM_IEEE80211_HW_FLAGS] != (void
> *)0x1);
> 
> It seems, that compiler instrumentation causes some code
> deoptimizations.
> Because of that GCC is not being able to resolve condition in
> BUILD_BUG_ON()
> at compile time.
> 
> We could make size of hw_flag_names array unspecified and replace the
> condition in BUILD_BUG_ON() with following:
> 	ARRAY_SIZE(hw_flag_names) != NUM_IEEE80211_HW_FLAGS
> 
> That will have the same effect as before (adding new flag without
> updating
> array will trigger build failure) except it doesn't fail with
> CONFIG_UBSAN.
> As a bonus this patch slightly decreases size of hw_flag_names array.
> 
Seems fine, would you want to take it through some other tree together
with UBSAN, or do you expect that to still take long enough to allow
this to trickle through our trees?

johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283272 — Re: [PATCH v4 2/3] mac80211: Prevent build failure with CONFIG_UBSAN=y

2015-12-03 20:05 GMT+03:00 Johannes Berg <johannes@sipsolutions.net>:
> On Thu, 2015-12-03 at 18:50 +0300, Andrey Ryabinin wrote:
>> With upcoming CONFIG_UBSAN the following BUILD_BUG_ON in
>> net/mac80211/debugfs.c starts to trigger:
>>       BUILD_BUG_ON(hw_flag_names[NUM_IEEE80211_HW_FLAGS] != (void
>> *)0x1);
>>
>> It seems, that compiler instrumentation causes some code
>> deoptimizations.
>> Because of that GCC is not being able to resolve condition in
>> BUILD_BUG_ON()
>> at compile time.
>>
>> We could make size of hw_flag_names array unspecified and replace the
>> condition in BUILD_BUG_ON() with following:
>>       ARRAY_SIZE(hw_flag_names) != NUM_IEEE80211_HW_FLAGS
>>
>> That will have the same effect as before (adding new flag without
>> updating
>> array will trigger build failure) except it doesn't fail with
>> CONFIG_UBSAN.
>> As a bonus this patch slightly decreases size of hw_flag_names array.
>>
> Seems fine, would you want to take it through some other tree together
> with UBSAN, or do you expect that to still take long enough to allow
> this to trickle through our trees?
>

I expect that Andrew will take it with UBSAN for 4.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1284401 — Re: [PATCH v4 0/3] UBSAN: run-time undefined behavior sanity checker

On Thu, 3 Dec 2015 18:50:04 +0300 Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:

> UBSAN is run-time undefined behaviour checker. It uses compile-time
> instrumentation to catch undefined behavior (UB). Compiler inserts code
> that perform certain kinds of checks before operations that could cause UB.
> If check fails (i.e. UB detected) __ubsan_handle_* function called to print error message.

What I'd like to see in this changelog is a description of any kernel
issues which this checker has already identified: what were they and
what was their potential impact at runtime.

This info will help us to understand the value of the proposed feature.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1285793

On 12/05/2015 03:37 AM, Andrew Morton wrote:
> On Thu, 3 Dec 2015 18:50:04 +0300 Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
> 
>> UBSAN is run-time undefined behaviour checker. It uses compile-time
>> instrumentation to catch undefined behavior (UB). Compiler inserts code
>> that perform certain kinds of checks before operations that could cause UB.
>> If check fails (i.e. UB detected) __ubsan_handle_* function called to print error message.
> 
> What I'd like to see in this changelog is a description of any kernel
> issues which this checker has already identified: what were they and
> what was their potential impact at runtime.
> 
> This info will help us to understand the value of the proposed feature.
> 

Sure, I'll come back soon with a list of found bugs.

> Thanks.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288618

On 12/05/2015 03:37 AM, Andrew Morton wrote:
> On Thu, 3 Dec 2015 18:50:04 +0300 Andrey Ryabinin <aryabinin@virtuozzo.com> wrote:
> 
>> UBSAN is run-time undefined behaviour checker. It uses compile-time
>> instrumentation to catch undefined behavior (UB). Compiler inserts code
>> that perform certain kinds of checks before operations that could cause UB.
>> If check fails (i.e. UB detected) __ubsan_handle_* function called to print error message.
> 
> What I'd like to see in this changelog is a description of any kernel
> issues which this checker has already identified: what were they and
> what was their potential impact at runtime.
> 
> This info will help us to understand the value of the proposed feature.
> 


It's hard for me to judge about runtime impact of those bugs,
as don't know much about what that bugged code is doing.
Frankly speaking, nothing looks too scary for me, although some bugs would
be hard to find without UBSAN.


Found bugs:

	* out-of-bounds access - 97840cb67ff5 ("netfilter: nfnetlink: fix insufficient validation in nfnetlink_bind")

undefined shifts:
	* d48458d4a768 ("jbd2: use a better hash function for the revoke table")
	* 10632008b9e1 ("clockevents: Prevent shift out of bounds")
	* 'x << -1' shift in ext4 - http://lkml.kernel.org/r/<5444EF21.8020501@samsung.com>
	* undefined rol32(0) - http://lkml.kernel.org/r/<1449198241-20654-1-git-send-email-sasha.levin@oracle.com>
	* undefined dirty_ratelimit calculation - http://lkml.kernel.org/r/<566594E2.3050306@odin.com>
	* undefined roundown_pow_of_two(0) - http://lkml.kernel.org/r/<1449156616-11474-1-git-send-email-sasha.levin@oracle.com>
	* [WONTFIX] undefined shift in __bpf_prog_run - http://lkml.kernel.org/r/<CACT4Y+ZxoR3UjLgcNdUm4fECLMx2VdtfrENMtRRCdgHB2n0bJA@mail.gmail.com>
		WONTFIX here because it should be fixed in bpf program, not in kernel.

signed overflows:
	* 32a8df4e0b33f ("sched: Fix odd values in effective_load() calculations")
	* mul overflow in ntp - http://lkml.kernel.org/r/<1449175608-1146-1-git-send-email-sasha.levin@oracle.com>
	* incorrect conversion into rtc_time in rtc_time64_to_tm() - http://lkml.kernel.org/r/<1449187944-11730-1-git-send-email-sasha.levin@oracle.com>
	* unvalidated timespec in io_getevents() - http://lkml.kernel.org/r/<CACT4Y+bBxVYLQ6LtOKrKtnLthqLHcw-BMp3aqP3mjdAvr9FULQ@mail.gmail.com>
	* [NOTABUG] signed overflow in ktime_add_safe() - http://lkml.kernel.org/r/<CACT4Y+aJ4muRnWxsUe1CMnA6P8nooO33kwG-c8YZg=0Xc8rJqw@mail.gmail.com>
		

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web