Skip to contentSkip to conversation

Undefined Behaviour Sanititizer support

PATCH New Ben Hutchings 1 participant 24 messages Dec 9, 2016

Subsystems (35)
lore
Share & tools
lore
#0 lore raw

cover Undefined Behaviour Sanititizer support

This is a backport of changes in 4.5 to add support for GCC's Undefined Behaviour Sanitizer (UBSAN), and fixes for undefined behaviour that it has found (that are not already in 4.4.30).

UBSAN detects (at run time) many operations that will result in
undefined behaviour, i.e. unpredictable results. These include overflow
of signed integer calculations, bitwise shifts by too many places, and
some array bounds errors. (KASAN detects other kinds of address
errors, but is currently only supported on arm64 and x86_64.) See
<http://developerblog.redhat.com/2014/10/16/gcc-undefined-behavior-sanitizer-ubsan/> for more information.

UBSAN can be enabled globally on x86 (CONFIG_UBSAN_SANITIZE_ALL), but must be enabled selectively on arm (add 'UBSAN_SANITIZE := y' to the Makefile in each directory you want to apply it to). I assume there are a few critical regions of code that must to be avoided.
It has a significant performance cost, so it is unlikely to be suitable for use in production builds.

For this and other backports, I'm looking for feedback like:
- Did I miss a follow-up fix or an earlier dependency?
- Does this cause a regression (other than as explained above)?
- Are you likely to use it?
- Are there related features you want in 4.4?

Ben.

Adam Borowski (2):
perf/x86/amd: Set the size of event map array to PERF_COUNT_HW_MAX
btrfs: fix int32 overflow in shrink_delalloc().

Andrey Ryabinin (3):

  UBSAN: run-time undefined behavior sanity checker
  mm/page-writeback: fix dirty_ratelimit calculation
  ubsan: fix tree-wide -Wmaybe-uninitialized false positives

Bartlomiej Zolnierkiewicz (1):
blk-mq: fix undefined behaviour in order_to_size()

Chris Bainbridge (1):
x86/microcode/intel: Change checksum variables to u32

David Decotigny (1):
mlx4: remove unused fields

Eric Biggers (1):
PM / sleep: declare __tracedata symbols as char[] rather than char

Eric Dumazet (1):
net: get rid of an signed integer overflow in ip_idents_reserve()

Nicolai Stange (2):
mm/filemap: generic_file_read_iter(): check for zero reads
unconditionally
drm/radeon: don't include RADEON_HPD_NONE in HPD IRQ enable bitsets

Nicolas Iooss (1):

  UBSAN: fix typo in format string

Oleg Nesterov (1):
signal: move the "sig < SIGRTMIN" check into siginmask(sig)

Peter Zijlstra (1):
perf/core: Fix Undefined behaviour in rb_alloc()

Seung-Woo Kim (2):
mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
pwm: samsung: Fix to use lowest div for large enough modulation bits

Sven Eckelmann (1):
batman-adv: Fix integer overflow in batadv_iv_ogm_calc_tq

Vegard Nossum (2):
rhashtable: fix shift by 64 when shrinking
time: Avoid undefined behaviour in ktime_add_safe()

Xie XiuQi (2):
drm: fix signed integer overflow
xfs: fix signed integer overflow

Yang Shi (1):
ubsan: cosmetic fix to Kconfig text

 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/kernel/cpu/microcode/intel_lib.c |   8 +-
 arch/x86/kernel/cpu/perf_event_amd.c      |   2 +-
 arch/x86/realmode/rm/Makefile             |   1 +
 block/blk-mq.c                            |   2 +-
 drivers/base/power/trace.c                |   4 +-
 drivers/firmware/efi/libstub/Makefile     |   1 +
 drivers/gpu/drm/drm_hashtab.c             |   2 +-
 drivers/gpu/drm/radeon/evergreen.c        |   6 +-
 drivers/gpu/drm/radeon/r100.c             |   6 +-
 drivers/gpu/drm/radeon/r600.c             |   6 +-
 drivers/gpu/drm/radeon/rs600.c            |   6 +-
 drivers/mmc/host/dw_mmc.c                 |  14 +-
 drivers/mmc/host/dw_mmc.h                 |   4 -
 drivers/net/ethernet/mellanox/mlx4/fw.c   |   4 -
 drivers/net/ethernet/mellanox/mlx4/fw.h   |   2 -
 drivers/pwm/pwm-samsung.c                 |  15 +-
 fs/btrfs/extent-tree.c                    |   2 +-
 fs/xfs/xfs_buf_item.c                     |   4 +-
 include/linux/ktime.h                     |   7 +
 include/linux/sched.h                     |   3 +
 include/linux/signal.h                    |  16 +-
 kernel/events/ring_buffer.c               |   6 +-
 kernel/time/hrtimer.c                     |   2 +-
 lib/Kconfig.debug                         |   1 +
 lib/Kconfig.ubsan                         |  36 +++
 lib/Makefile                              |   3 +
 lib/rhashtable.c                          |   6 +-
 lib/ubsan.c                               | 456 ++++++++++++++++++++++++++++++
 lib/ubsan.h                               |  84 ++++++
 mm/filemap.c                              |   7 +-
 mm/kasan/Makefile                         |   1 +
 mm/page-writeback.c                       |  11 +-
 net/batman-adv/bat_iv_ogm.c               |   5 +-
 net/ipv4/route.c                          |  10 +-
 scripts/Makefile.lib                      |   6 +
 scripts/Makefile.ubsan                    |  21 ++
 42 files changed, 792 insertions(+), 69 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
Signature · 6 lines
2.10.2


-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#1 lore raw

PATCH 1/23 UBSAN: run-time undefined behavior sanity checker

From: Andrey Ryabinin <[email protected]>

commit c6d308534aef6c99904bf5862066360ae067abc4 upstream.

UBSAN 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.

So the most of the work is done by compiler. This patch just implements ubsan handlers printing errors.

GCC has this capability since 4.9.x [1] (see -fsanitize=undefined option and its suboptions).
However GCC 5.x has more checkers implemented [2]. Article [3] has a bit more details about UBSAN in the GCC.

[1] - https://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Debugging-Options.html
[2] - https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html
[3] - http://developerblog.redhat.com/2014/10/16/gcc-undefined-behavior-sanitizer-ubsan/

Issues which UBSAN has found thus far are:

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/<[email protected]>

* undefined rol32(0) -
http://lkml.kernel.org/r/<[email protected]>

* undefined dirty_ratelimit calculation -
http://lkml.kernel.org/r/<[email protected]>

* undefined roundown_pow_of_two(0) -
http://lkml.kernel.org/r/<[email protected]>

* [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/<[email protected]>

* incorrect conversion into rtc_time in rtc_time64_to_tm() -
http://lkml.kernel.org/r/<[email protected]>

* 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>

[akpm at linux-foundation.org: fix unused local warning]
[akpm at linux-foundation.org: fix __int128 build woes]
Cc Peter Zijlstra <[email protected]>
Cc Sasha Levin <[email protected]>
Cc Randy Dunlap <[email protected]>
Cc Rasmus Villemoes <[email protected]>
Cc Jonathan Corbet <[email protected]>
Cc Michal Marek <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Yury Gribov <[email protected]>
Cc Dmitry Vyukov <[email protected]>
Cc Konstantin Khlebnikov <[email protected]>
Cc Kostya Serebryany <[email protected]>
Cc Johannes Berg <[email protected]>
---
 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 +
 lib/Kconfig.debug                     |   1 +
 lib/Kconfig.ubsan                     |  29 +++
 lib/Makefile                          |   3 +
 lib/ubsan.c                           | 456 ++++++++++++++++++++++++++++++++++
 lib/ubsan.h                           |  84 +++++++
 mm/kasan/Makefile                     |   1 +
 scripts/Makefile.lib                  |   6 +
 scripts/Makefile.ubsan                |  17 ++
 17 files changed, 692 insertions(+), 1 deletion(-)
 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
Documentation/ubsan.txt new file +84 -0
new file mode 100644
index 000000000000..f58215ef5797
@@ -0,0 +1,84 @@
+Undefined Behavior Sanitizer - UBSAN
+
+Overview
+--------
+
+UBSAN is a runtime undefined behaviour checker.
+
+UBSAN uses compile-time instrumentation to catch undefined behavior (UB).
+Compiler inserts code that perform certain kinds of checks before operations
+that may cause UB. If check fails (i.e. UB detected) __ubsan_handle_*
+function called to print error message.
+
+GCC has that feature since 4.9.x [1] (see -fsanitize=undefined option and
+its suboptions). GCC 5.x has more checkers implemented [2].
+
+Report example
+---------------
+
+	 ================================================================================
+	 UBSAN: Undefined behaviour in ../include/linux/bitops.h:110:33
+	 shift exponent 32 is to large for 32-bit type 'unsigned int'
+	 CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc1+ #26
+	  0000000000000000 ffffffff82403cc8 ffffffff815e6cd6 0000000000000001
+	  ffffffff82403cf8 ffffffff82403ce0 ffffffff8163a5ed 0000000000000020
+	  ffffffff82403d78 ffffffff8163ac2b ffffffff815f0001 0000000000000002
+	 Call Trace:
+	  [<ffffffff815e6cd6>] dump_stack+0x45/0x5f
+	  [<ffffffff8163a5ed>] ubsan_epilogue+0xd/0x40
+	  [<ffffffff8163ac2b>] __ubsan_handle_shift_out_of_bounds+0xeb/0x130
+	  [<ffffffff815f0001>] ? radix_tree_gang_lookup_slot+0x51/0x150
+	  [<ffffffff8173c586>] _mix_pool_bytes+0x1e6/0x480
+	  [<ffffffff83105653>] ? dmi_walk_early+0x48/0x5c
+	  [<ffffffff8173c881>] add_device_randomness+0x61/0x130
+	  [<ffffffff83105b35>] ? dmi_save_one_device+0xaa/0xaa
+	  [<ffffffff83105653>] dmi_walk_early+0x48/0x5c
+	  [<ffffffff831066ae>] dmi_scan_machine+0x278/0x4b4
+	  [<ffffffff8111d58a>] ? vprintk_default+0x1a/0x20
+	  [<ffffffff830ad120>] ? early_idt_handler_array+0x120/0x120
+	  [<ffffffff830b2240>] setup_arch+0x405/0xc2c
+	  [<ffffffff830ad120>] ? early_idt_handler_array+0x120/0x120
+	  [<ffffffff830ae053>] start_kernel+0x83/0x49a
+	  [<ffffffff830ad120>] ? early_idt_handler_array+0x120/0x120
+	  [<ffffffff830ad386>] x86_64_start_reservations+0x2a/0x2c
+	  [<ffffffff830ad4f3>] x86_64_start_kernel+0x16b/0x17a
+	 ================================================================================
+
+Usage
+-----
+
+To enable UBSAN configure kernel with:
+
+	CONFIG_UBSAN=y
+
+and to check the entire kernel:
+
+        CONFIG_UBSAN_SANITIZE_ALL=y
+
+To enable instrumentation for specific files or directories, add a line
+similar to the following to the respective kernel Makefile:
+
+        For a single file (e.g. main.o):
+                UBSAN_SANITIZE_main.o := y
+
+        For all files in one directory:
+                UBSAN_SANITIZE := y
+
+To exclude files from being instrumented even if
+CONFIG_UBSAN_SANITIZE_ALL=y, use:
+
+                UBSAN_SANITIZE_main.o := n
+        and:
+                UBSAN_SANITIZE := n
+
+Detection of unaligned accesses controlled through the separate option -
+CONFIG_UBSAN_ALIGNMENT. It's off by default on architectures that support
+unaligned accesses (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y). One could
+still enable it in config, just note that it will produce a lot of UBSAN
+reports.
+
+References
+----------
+
+[1] - https://gcc.gnu.org/onlinedocs/gcc-4.9.0/gcc/Debugging-Options.html
+[2] - https://gcc.gnu.org/onlinedocs/gcc/Debugging-Options.html
Makefile +2 -1
index 98239d56924c..504a8b90cf4e 100644
@@ -415,7 +415,7 @@ export MAKE AWK GENKSYMS INSTALLKERNEL PERL PYTHON UTS_MACHINE
 export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
 
 export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
-export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE CFLAGS_GCOV CFLAGS_KASAN
+export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE CFLAGS_GCOV CFLAGS_KASAN CFLAGS_UBSAN
 export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE
 export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_LDFLAGS_MODULE
 export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
@@ -794,6 +794,7 @@ endif
 
 include scripts/Makefile.kasan
 include scripts/Makefile.extrawarn
+include scripts/Makefile.ubsan
 
 # Add any arch overrides and user supplied CPPFLAGS, AFLAGS and CFLAGS as the
 # last assignments
arch/x86/Kconfig +1 -0
index 436639a31624..803b262eb0bf 100644
@@ -30,6 +30,7 @@ config X86
 	select ARCH_HAS_PMEM_API		if X86_64
 	select ARCH_HAS_MMIO_FLUSH
 	select ARCH_HAS_SG_CHAIN
+	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
 	select ARCH_MIGHT_HAVE_ACPI_PDC		if ACPI
 	select ARCH_MIGHT_HAVE_PC_PARPORT
arch/x86/boot/Makefile +1 -0
index c0cc2a6be0bf..4a3a140f26bf 100644
@@ -60,6 +60,7 @@ clean-files += cpustr.h
 KBUILD_CFLAGS	:= $(USERINCLUDE) $(REALMODE_CFLAGS) -D_SETUP
 KBUILD_AFLAGS	:= $(KBUILD_CFLAGS) -D__ASSEMBLY__
 GCOV_PROFILE := n
+UBSAN_SANITIZE := n
 
 $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
 
arch/x86/boot/compressed/Makefile +1 -0
index efa6073ffa7e..7f6c157e5da5 100644
@@ -33,6 +33,7 @@ KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
 
 KBUILD_AFLAGS  := $(KBUILD_CFLAGS) -D__ASSEMBLY__
 GCOV_PROFILE := n
+UBSAN_SANITIZE :=n
 
 LDFLAGS := -m elf_$(UTS_MACHINE)
 ifeq ($(CONFIG_RELOCATABLE),y)
arch/x86/entry/vdso/Makefile +1 -0
index 265c0ed68118..c854541d93ff 100644
@@ -4,6 +4,7 @@
 
 KBUILD_CFLAGS += $(DISABLE_LTO)
 KASAN_SANITIZE := n
+UBSAN_SANITIZE := n
 
 VDSO64-$(CONFIG_X86_64)		:= y
 VDSOX32-$(CONFIG_X86_X32_ABI)	:= y
arch/x86/realmode/rm/Makefile +1 -0
index 2730d775ef9a..3e75fcf6b836 100644
@@ -70,3 +70,4 @@ KBUILD_CFLAGS	:= $(LINUXINCLUDE) $(REALMODE_CFLAGS) -D_SETUP -D_WAKEUP \
 		   -I$(srctree)/arch/x86/boot
 KBUILD_AFLAGS	:= $(KBUILD_CFLAGS) -D__ASSEMBLY__
 GCOV_PROFILE := n
+UBSAN_SANITIZE := n
drivers/firmware/efi/libstub/Makefile +1 -0
index 3c0467d3688c..b2a172d93a08 100644
@@ -22,6 +22,7 @@ KBUILD_CFLAGS			:= $(cflags-y) \
 
 GCOV_PROFILE			:= n
 KASAN_SANITIZE			:= n
+UBSAN_SANITIZE			:= n
 
 lib-y				:= efi-stub-helper.o
 
include/linux/sched.h +3 -0
index 1c0193baea2a..90e11e40219a 100644
@@ -1632,6 +1632,9 @@ struct task_struct {
 	struct held_lock held_locks[MAX_LOCK_DEPTH];
 	gfp_t lockdep_reclaim_gfp;
 #endif
+#ifdef CONFIG_UBSAN
+	unsigned int in_ubsan;
+#endif
 
 /* journalling filesystem info */
 	void *journal_info;
lib/Kconfig.debug +1 -0
index 8c15b29d5adc..4bab98fe59a4 100644
@@ -1853,3 +1853,4 @@ source "samples/Kconfig"
 
 source "lib/Kconfig.kgdb"
 
+source "lib/Kconfig.ubsan"
lib/Kconfig.ubsan new file +29 -0
new file mode 100644
index 000000000000..49518fb48cab
@@ -0,0 +1,29 @@
+config ARCH_HAS_UBSAN_SANITIZE_ALL
+	bool
+
+config UBSAN
+	bool "Undefined behaviour sanity checker"
+	help
+	  This option enables undefined behaviour sanity checker
+	  Compile-time instrumentation is used to detect various undefined
+	  behaviours in runtime. Various types of checks may be enabled
+	  via boot parameter ubsan_handle (see: Documentation/ubsan.txt).
+
+config UBSAN_SANITIZE_ALL
+	bool "Enable instrumentation for the entire kernel"
+	depends on UBSAN
+	depends on ARCH_HAS_UBSAN_SANITIZE_ALL
+	default y
+	help
+	  This option activates instrumentation for the entire kernel.
+	  If you don't enable this option, you have to explicitly specify
+	  UBSAN_SANITIZE := y for the files/directories you want to check for UB.
+
+config UBSAN_ALIGNMENT
+	bool "Enable checking of pointers alignment"
+	depends on UBSAN
+	default y if !HAVE_EFFICIENT_UNALIGNED_ACCESS
+	help
+	  This option enables detection of unaligned memory accesses.
+	  Enabling this option on architectures that support unalligned
+	  accesses may produce a lot of false positives.
lib/Makefile +3 -0
index 7f1de26613d2..e710b082d38f 100644
@@ -208,3 +208,6 @@ quiet_cmd_build_OID_registry = GEN     $@
 clean-files	+= oid_registry_data.c
 
 obj-$(CONFIG_UCS2_STRING) += ucs2_string.o
+obj-$(CONFIG_UBSAN) += ubsan.o
+
+UBSAN_SANITIZE_ubsan.o := n
lib/ubsan.c new file +456 -0

456 lines not on this page · raw message

lib/ubsan.h new file +84 -0

84 lines not on this page · raw message

mm/kasan/Makefile +1 -0
index 64710148941e..a61460d9f5b0 100644
@@ -1,4 +1,5 @@
 KASAN_SANITIZE := n
+UBSAN_SANITIZE_kasan.o := n
 
 CFLAGS_REMOVE_kasan.o = -pg
 # Function splitter causes unnecessary splits in __asan_load1/__asan_store1
scripts/Makefile.lib +6 -0
index 79e86613712f..d7c2032d0fe4 100644
@@ -129,6 +129,12 @@ _c_flags += $(if $(patsubst n%,, \
 		$(CFLAGS_KASAN))
 endif
 
+ifeq ($(CONFIG_UBSAN),y)
+_c_flags += $(if $(patsubst n%,, \
+		$(UBSAN_SANITIZE_$(basetarget).o)$(UBSAN_SANITIZE)$(CONFIG_UBSAN_SANITIZE_ALL)), \
+		$(CFLAGS_UBSAN))
+endif
+
 # If building the kernel in a separate objtree expand all occurrences
 # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/').
 
scripts/Makefile.ubsan new file +17 -0
new file mode 100644
index 000000000000..8ab68679cfb5
@@ -0,0 +1,17 @@
+ifdef CONFIG_UBSAN
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=shift)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=integer-divide-by-zero)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=unreachable)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=vla-bound)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=null)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=signed-integer-overflow)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=bounds)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=object-size)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=returns-nonnull-attribute)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=bool)
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=enum)
+
+ifdef CONFIG_UBSAN_ALIGNMENT
+      CFLAGS_UBSAN += $(call cc-option, -fsanitize=alignment)
+endif
+endif
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#2 lore raw

PATCH 2/23 ubsan: cosmetic fix to Kconfig text

In reply to Ben Hutchings

From: Yang Shi <[email protected]>

commit 7707535ab95e2231b6d7f2bfb4f27558e83c4dc2 upstream.

When enabling UBSAN_SANITIZE_ALL, the kernel image size gets increased significantly (~3x). So, it sounds better to have some note in Kconfig.

And, fixed a typo.

---
 lib/Kconfig.ubsan | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
lib/Kconfig.ubsan +3 -1
index 49518fb48cab..e07c1ba9ba13 100644
@@ -18,6 +18,8 @@ config UBSAN_SANITIZE_ALL
 	  This option activates instrumentation for the entire kernel.
 	  If you don't enable this option, you have to explicitly specify
 	  UBSAN_SANITIZE := y for the files/directories you want to check for UB.
+	  Enabling this option will get kernel image size increased
+	  significantly.
 
 config UBSAN_ALIGNMENT
 	bool "Enable checking of pointers alignment"
@@ -25,5 +27,5 @@ config UBSAN_ALIGNMENT
 	default y if !HAVE_EFFICIENT_UNALIGNED_ACCESS
 	help
 	  This option enables detection of unaligned memory accesses.
-	  Enabling this option on architectures that support unalligned
+	  Enabling this option on architectures that support unaligned
 	  accesses may produce a lot of false positives.
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#3 lore raw

PATCH 3/23 PM / sleep: declare __tracedata symbols as char[] rather than char

In reply to Ben Hutchings

From: Eric Biggers <[email protected]>

commit f97238373b8662a6d580e204df2e7bcbfa43e27a upstream.

Accessing more than one byte from a symbol declared simply 'char' is undefined behavior, as reported by UBSAN:

	UBSAN: Undefined behaviour in drivers/base/power/trace.c:178:18
	load of address ffffffff8203fc78 with insufficient space
	for an object of type 'char'

Avoid this by declaring the symbols as arrays.

---
 drivers/base/power/trace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/base/power/trace.c +2 -2
index a311cfa4c5bd..a6975795e7f3 100644
@@ -166,14 +166,14 @@ void generate_pm_trace(const void *tracedata, unsigned int user)
 }
 EXPORT_SYMBOL(generate_pm_trace);
 
-extern char __tracedata_start, __tracedata_end;
+extern char __tracedata_start[], __tracedata_end[];
 static int show_file_hash(unsigned int value)
 {
 	int match;
 	char *tracedata;
 
 	match = 0;
-	for (tracedata = &__tracedata_start ; tracedata < &__tracedata_end ;
+	for (tracedata = __tracedata_start ; tracedata < __tracedata_end ;
 			tracedata += 2 + sizeof(unsigned long)) {
 		unsigned short lineno = *(unsigned short *)tracedata;
 		const char *file = *(const char **)(tracedata + 2);
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#4 lore raw

PATCH 4/23 x86/microcode/intel: Change checksum variables to u32

In reply to Ben Hutchings

From: Chris Bainbridge <[email protected]>

commit bc864af13f34d19c911f5691d87bdacc9ce109f5 upstream.

Microcode checksum verification should be done using unsigned 32-bit values otherwise the calculation overflow results in undefined behaviour.

This is also nicely documented in the SDM, section "Microcode Update Checksum":

"To check for a corrupt microcode update, software must perform a unsigned DWORD (32-bit) checksum of the microcode update. Even though some fields are signed, the checksum procedure treats all DWORDs as unsigned. Microcode updates with a header version equal to 00000001H must sum all DWORDs that comprise the microcode update. A valid checksum check will yield a value of 00000000H."

but for some reason the code has been using ints from the very beginning.

In practice, this bug possibly manifested itself only when doing the microcode data checksum - apparently, currently shipped Intel microcode doesn't have an extended signature table for which we do checksum verification too.

  UBSAN: Undefined behaviour in arch/x86/kernel/cpu/microcode/intel_lib.c:105:12
  signed integer overflow:
  -1500151068 + -2125470173 cannot be represented in type 'int'
  CPU: 0 PID: 0 Comm: swapper Not tainted 4.5.0-rc5+ #495
  ...
  Call Trace:
   dump_stack
   ? inotify_ioctl
   ubsan_epilogue
   handle_overflow
   __ubsan_handle_add_overflow
   microcode_sanity_check
   get_matching_model_microcode.isra.2.constprop.8
   ? early_idt_handler_common
   ? strlcpy
   ? find_cpio_data
   load_ucode_intel_bsp
   load_ucode_bsp
   ? load_ucode_bsp
   x86_64_start_kernel
---
 arch/x86/kernel/cpu/microcode/intel_lib.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/kernel/cpu/microcode/intel_lib.c +4 -4
index b96896bcbdaf..99ca2c935777 100644
@@ -49,7 +49,7 @@ int microcode_sanity_check(void *mc, int print_err)
 	unsigned long total_size, data_size, ext_table_size;
 	struct microcode_header_intel *mc_header = mc;
 	struct extended_sigtable *ext_header = NULL;
-	int sum, orig_sum, ext_sigcount = 0, i;
+	u32 sum, orig_sum, ext_sigcount = 0, i;
 	struct extended_signature *ext_sig;
 
 	total_size = get_totalsize(mc_header);
@@ -85,8 +85,8 @@ int microcode_sanity_check(void *mc, int print_err)
 
 	/* check extended table checksum */
 	if (ext_table_size) {
-		int ext_table_sum = 0;
-		int *ext_tablep = (int *)ext_header;
+		u32 ext_table_sum = 0;
+		u32 *ext_tablep = (u32 *)ext_header;
 
 		i = ext_table_size / DWSIZE;
 		while (i--)
@@ -102,7 +102,7 @@ int microcode_sanity_check(void *mc, int print_err)
 	orig_sum = 0;
 	i = (MC_HEADER_SIZE + data_size) / DWSIZE;
 	while (i--)
-		orig_sum += ((int *)mc)[i];
+		orig_sum += ((u32 *)mc)[i];
 	if (orig_sum) {
 		if (print_err)
 			pr_err("aborting, bad checksum\n");
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#5 lore raw

PATCH 5/23 mm/page-writeback: fix dirty_ratelimit calculation

In reply to Ben Hutchings

From: Andrey Ryabinin <[email protected]>

commit d59b1087a98e402ed9a7cc577f4da435f9a555f5 upstream.

Calculation of dirty_ratelimit sometimes is not correct. E.g. initial values of dirty_ratelimit == INIT_BW and step == 0, lead to the following result:

   UBSAN: Undefined behaviour in ../mm/page-writeback.c:1286:7
   shift exponent 25600 is too large for 64-bit type 'long unsigned int'

The fix is straightforward - make step 0 if the shift exponent is too big.

---
 mm/page-writeback.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
mm/page-writeback.c +6 -5
index fd51ebfc423f..6d0dbde4503b 100644
@@ -1162,6 +1162,7 @@ static void wb_update_dirty_ratelimit(struct dirty_throttle_control *dtc,
 	unsigned long balanced_dirty_ratelimit;
 	unsigned long step;
 	unsigned long x;
+	unsigned long shift;
 
 	/*
 	 * The dirty rate will match the writeout rate in long term, except
@@ -1286,11 +1287,11 @@ static void wb_update_dirty_ratelimit(struct dirty_throttle_control *dtc,
 	 * rate itself is constantly fluctuating. So decrease the track speed
 	 * when it gets close to the target. Helps eliminate pointless tremors.
 	 */
-	step >>= dirty_ratelimit / (2 * step + 1);
-	/*
-	 * Limit the tracking speed to avoid overshooting.
-	 */
-	step = (step + 7) / 8;
+	shift = dirty_ratelimit / (2 * step + 1);
+	if (shift < BITS_PER_LONG)
+		step = DIV_ROUND_UP(step >> shift, 8);
+	else
+		step = 0;
 
 	if (dirty_ratelimit < balanced_dirty_ratelimit)
 		dirty_ratelimit += step;
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#6 lore raw

PATCH 6/23 perf/core: Fix Undefined behaviour in rb_alloc()

In reply to Ben Hutchings

From: Peter Zijlstra <[email protected]>

commit 8184059e93c200757f5c0805dae0f14e880eab5d upstream.

Sasha reported:

 [ 3494.030114] UBSAN: Undefined behaviour in kernel/events/ring_buffer.c:685:22
 [ 3494.030647] shift exponent -1 is negative

Andrey spotted that this is because:

It happens if nr_pages = 0:

     rb->page_order = ilog2(nr_pages);

Fix it by making both assignments conditional on nr_pages; since otherwise they should both be 0 anyway, and will be because of the kzalloc() used to allocate the structure.

Cc Alexander Shishkin <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc David Ahern <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Vince Weaver <[email protected]>
---
 kernel/events/ring_buffer.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/events/ring_buffer.c +4 -2
index 014b69528194..7beb02f40cb4 100644
@@ -754,8 +754,10 @@ struct ring_buffer *rb_alloc(int nr_pages, long watermark, int cpu, int flags)
 
 	rb->user_page = all_buf;
 	rb->data_pages[0] = all_buf + PAGE_SIZE;
-	rb->page_order = ilog2(nr_pages);
-	rb->nr_pages = !!nr_pages;
+	if (nr_pages) {
+		rb->nr_pages = 1;
+		rb->page_order = ilog2(nr_pages);
+	}
 
 	ring_buffer_init(rb, watermark, flags);
 
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#7 lore raw

PATCH 7/23 ubsan: fix tree-wide -Wmaybe-uninitialized false positives

In reply to Ben Hutchings

From: Andrey Ryabinin <[email protected]>

commit dde5cf39d4d2cce71f2997c37210dd624d0e4bf6 upstream.

-fsanitize=* options makes GCC less smart than usual and increase number of 'maybe-uninitialized' false-positives. So this patch does two things:

* Add -Wno-maybe-uninitialized to CFLAGS_UBSAN which will disable all such warnings for instrumented files.

* Remove CONFIG_UBSAN_SANITIZE_ALL from all[yes|mod]config builds. So the all[yes|mod]config build goes without -fsanitize=* and still with -Wmaybe-uninitialized.

---
 lib/Kconfig.ubsan      | 5 +++++
 scripts/Makefile.ubsan | 4 ++++
 2 files changed, 9 insertions(+)
lib/Kconfig.ubsan +5 -0
index e07c1ba9ba13..39494af9a84a 100644
@@ -13,6 +13,11 @@ config UBSAN_SANITIZE_ALL
 	bool "Enable instrumentation for the entire kernel"
 	depends on UBSAN
 	depends on ARCH_HAS_UBSAN_SANITIZE_ALL
+
+	# We build with -Wno-maybe-uninitilzed, but we still want to
+	# use -Wmaybe-uninitilized in allmodconfig builds.
+	# So dependsy bellow used to disable this option in allmodconfig
+	depends on !COMPILE_TEST
 	default y
 	help
 	  This option activates instrumentation for the entire kernel.
scripts/Makefile.ubsan +4 -0
index 8ab68679cfb5..77ce538268b5 100644
@@ -14,4 +14,8 @@ ifdef CONFIG_UBSAN
 ifdef CONFIG_UBSAN_ALIGNMENT
       CFLAGS_UBSAN += $(call cc-option, -fsanitize=alignment)
 endif
+
+      # -fsanitize=* options makes GCC less smart than usual and
+      # increase number of 'maybe-uninitialized false-positives
+      CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
 endif
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#8 lore raw

PATCH 8/23 mm/filemap: generic_file_read_iter(): check for zero reads unconditionally

In reply to Ben Hutchings

From: Nicolai Stange <[email protected]>

commit e7080a439a6f507abbc860847c33efc39b5c1c6d upstream.

If
- generic_file_read_iter() gets called with a zero read length,
- the read offset is at a page boundary,
- IOCB_DIRECT is not set
- and the page in question hasn't made it into the page cache yet, then do_generic_file_read() will trigger a readahead with a req_size hint of zero.

Since roundup_pow_of_two(0) is undefined, UBSAN reports

  UBSAN: Undefined behaviour in include/linux/log2.h:63:13
  shift exponent 64 is too large for 64-bit type 'long unsigned int'
  CPU: 3 PID: 1017 Comm: sa1 Tainted: G L 4.5.0-next-20160318+ #14
  [...]
  Call Trace:
   [...]
   [<ffffffff813ef61a>] ondemand_readahead+0x3aa/0x3d0
   [<ffffffff813ef61a>] ? ondemand_readahead+0x3aa/0x3d0
   [<ffffffff813c73bd>] ? find_get_entry+0x2d/0x210
   [<ffffffff813ef9c3>] page_cache_sync_readahead+0x63/0xa0
   [<ffffffff813cc04d>] do_generic_file_read+0x80d/0xf90
   [<ffffffff813cc955>] generic_file_read_iter+0x185/0x420
   [...]
   [<ffffffff81510b06>] __vfs_read+0x256/0x3d0
   [...]

when get_init_ra_size() gets called from ondemand_readahead().

The net effect is that the initial readahead size is arch dependent for requested read lengths of zero: for example, since

1UL << (sizeof(unsigned long) * 8)

evaluates to 1 on x86 while its result is 0 on ARMv7, the initial readahead size becomes 4 on the former and 0 on the latter.

What's more, whether or not the file access timestamp is updated for zero length reads is decided differently for the two cases of IOCB_DIRECT being set or cleared: in the first case, generic_file_read_iter() explicitly skips updating that timestamp while in the latter case, it is always updated through the call to do_generic_file_read().

According to POSIX, zero length reads "do not modify the last data access timestamp" and thus, the IOCB_DIRECT behaviour is POSIXly correct.

Let generic_file_read_iter() unconditionally check the requested read length at its entry and return immediately with success if it is zero.

---
 mm/filemap.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
mm/filemap.c +4 -3
index c588d1222b2a..521bd370339c 100644
@@ -1766,15 +1766,16 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
 	ssize_t retval = 0;
 	loff_t *ppos = &iocb->ki_pos;
 	loff_t pos = *ppos;
+	size_t count = iov_iter_count(iter);
+
+	if (!count)
+		goto out; /* skip atime */
 
 	if (iocb->ki_flags & IOCB_DIRECT) {
 		struct address_space *mapping = file->f_mapping;
 		struct inode *inode = mapping->host;
-		size_t count = iov_iter_count(iter);
 		loff_t size;
 
-		if (!count)
-			goto out; /* skip atime */
 		size = i_size_read(inode);
 		retval = filemap_write_and_wait_range(mapping, pos,
 					pos + count - 1);
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#9 lore raw

PATCH 9/23 perf/x86/amd: Set the size of event map array to PERF_COUNT_HW_MAX

In reply to Ben Hutchings

From: Adam Borowski <[email protected]>

commit 0a25556f84d5f79e68e9502bb1f32a43377ab2bf upstream.

The entry for PERF_COUNT_HW_REF_CPU_CYCLES is not used on AMD, but is referenced by filter_events() which expects undefined events to have a value of 0.

Found via KASAN:

  UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:30
  index 9 is out of range for type 'u64 [9]'
  UBSAN: Undefined behaviour in arch/x86/events/amd/core.c:132:9
  load of address ffffffff81c021c8 with insufficient space for an object of type 'const u64'
Cc Alexander Shishkin <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Vince Weaver <[email protected]>
[bwh: Backported to 4.4: adjust filename]
---
 arch/x86/kernel/cpu/perf_event_amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/cpu/perf_event_amd.c +1 -1
index 1cee5d2d7ece..e671f3c2397e 100644
@@ -115,7 +115,7 @@ static __initconst const u64 amd_hw_cache_event_ids
 /*
  * AMD Performance Monitor K7 and later.
  */
-static const u64 amd_perfmon_event_map[] =
+static const u64 amd_perfmon_event_map[PERF_COUNT_HW_MAX] =
 {
   [PERF_COUNT_HW_CPU_CYCLES]			= 0x0076,
   [PERF_COUNT_HW_INSTRUCTIONS]			= 0x00c0,
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#10 lore raw

PATCH 10/23 drm/radeon: don't include RADEON_HPD_NONE in HPD IRQ enable bitsets

In reply to Ben Hutchings

From: Nicolai Stange <[email protected]>

commit b2c0cbd657173f024138d6421774007690ceeffd upstream.

The values of all but the RADEON_HPD_NONE members of the radeon_hpd_id enum transform 1:1 into bit positions within the 'enabled' bitset as assembled by evergreen_hpd_init():

  enabled |= 1 << radeon_connector->hpd.hpd;

However, if ->hpd.hpd happens to equal RADEON_HPD_NONE == 0xff, UBSAN reports

  UBSAN: Undefined behaviour in drivers/gpu/drm/radeon/evergreen.c:1867:16
  shift exponent 255 is too large for 32-bit type 'int'
  [...]
  Call Trace:
   [<ffffffff818c4d35>] dump_stack+0xbc/0x117
   [<ffffffff818c4c79>] ? _atomic_dec_and_lock+0x169/0x169
   [<ffffffff819411bb>] ubsan_epilogue+0xd/0x4e
   [<ffffffff81941cbc>] __ubsan_handle_shift_out_of_bounds+0x1fb/0x254
   [<ffffffffa0ba7f2e>] ? atom_execute_table+0x3e/0x50 [radeon]
   [<ffffffff81941ac1>] ? __ubsan_handle_load_invalid_value+0x158/0x158
   [<ffffffffa0b87700>] ? radeon_get_pll_use_mask+0x130/0x130 [radeon]
   [<ffffffff81219930>] ? wake_up_klogd_work_func+0x60/0x60
   [<ffffffff8121a35e>] ? vprintk_default+0x3e/0x60
   [<ffffffffa0c603c4>] evergreen_hpd_init+0x274/0x2d0 [radeon]
   [<ffffffffa0c603c4>] ? evergreen_hpd_init+0x274/0x2d0 [radeon]
   [<ffffffffa0bd196e>] radeon_modeset_init+0x8ce/0x18d0 [radeon]
   [<ffffffffa0b71d86>] radeon_driver_load_kms+0x186/0x350 [radeon]
   [<ffffffffa03b6b16>] drm_dev_register+0xc6/0x100 [drm]
   [<ffffffffa03bc8c4>] drm_get_pci_dev+0xe4/0x490 [drm]
   [<ffffffff814b83f0>] ? kfree+0x220/0x370
   [<ffffffffa0b687c2>] radeon_pci_probe+0x112/0x140 [radeon]
   [...]
  =====================================================================
  radeon 0000:01:00.0: No connectors reported connected with modes

At least on x86, there should be no user-visible impact as there

1 << 0xff == 1 << (0xff & 31) == 1 << 31

holds and 31 > RADEON_MAX_HPD_PINS. Thus, this patch is a cosmetic one.

All of the above applies analogously to evergreen_hpd_fini(), r100_hpd_init(), r100_hpd_fini(), r600_hpd_init(), r600_hpd_fini(), rs600_hpd_init() and rs600_hpd_fini()

Silence UBSAN by checking ->hpd.hpd for RADEON_HPD_NONE before oring it into the 'enabled' bitset in the *_init()- or the 'disabled' bitset in the *_fini()-functions respectively.

---
 drivers/gpu/drm/radeon/evergreen.c | 6 ++++--
 drivers/gpu/drm/radeon/r100.c      | 6 ++++--
 drivers/gpu/drm/radeon/r600.c      | 6 ++++--
 drivers/gpu/drm/radeon/rs600.c     | 6 ++++--
 4 files changed, 16 insertions(+), 8 deletions(-)
drivers/gpu/drm/radeon/evergreen.c +4 -2
index 32491355a1d4..6792df8ed01a 100644
@@ -1864,7 +1864,8 @@ void evergreen_hpd_init(struct radeon_device *rdev)
 			break;
 		}
 		radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd);
-		enabled |= 1 << radeon_connector->hpd.hpd;
+		if (radeon_connector->hpd.hpd != RADEON_HPD_NONE)
+			enabled |= 1 << radeon_connector->hpd.hpd;
 	}
 	radeon_irq_kms_enable_hpd(rdev, enabled);
 }
@@ -1907,7 +1908,8 @@ void evergreen_hpd_fini(struct radeon_device *rdev)
 		default:
 			break;
 		}
-		disabled |= 1 << radeon_connector->hpd.hpd;
+		if (radeon_connector->hpd.hpd != RADEON_HPD_NONE)
+			disabled |= 1 << radeon_connector->hpd.hpd;
 	}
 	radeon_irq_kms_disable_hpd(rdev, disabled);
 }
drivers/gpu/drm/radeon/r100.c +4 -2
index 9e7e2bf03b81..9c5f47099216 100644
@@ -592,7 +592,8 @@ void r100_hpd_init(struct radeon_device *rdev)
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 		struct radeon_connector *radeon_connector = to_radeon_connector(connector);
-		enable |= 1 << radeon_connector->hpd.hpd;
+		if (radeon_connector->hpd.hpd != RADEON_HPD_NONE)
+			enable |= 1 << radeon_connector->hpd.hpd;
 		radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd);
 	}
 	radeon_irq_kms_enable_hpd(rdev, enable);
@@ -614,7 +615,8 @@ void r100_hpd_fini(struct radeon_device *rdev)
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 		struct radeon_connector *radeon_connector = to_radeon_connector(connector);
-		disable |= 1 << radeon_connector->hpd.hpd;
+		if (radeon_connector->hpd.hpd != RADEON_HPD_NONE)
+			disable |= 1 << radeon_connector->hpd.hpd;
 	}
 	radeon_irq_kms_disable_hpd(rdev, disable);
 }
drivers/gpu/drm/radeon/r600.c +4 -2
index cc2fdf0be37a..116373c24d08 100644
@@ -1002,7 +1002,8 @@ void r600_hpd_init(struct radeon_device *rdev)
 				break;
 			}
 		}
-		enable |= 1 << radeon_connector->hpd.hpd;
+		if (radeon_connector->hpd.hpd != RADEON_HPD_NONE)
+			enable |= 1 << radeon_connector->hpd.hpd;
 		radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd);
 	}
 	radeon_irq_kms_enable_hpd(rdev, enable);
@@ -1055,7 +1056,8 @@ void r600_hpd_fini(struct radeon_device *rdev)
 				break;
 			}
 		}
-		disable |= 1 << radeon_connector->hpd.hpd;
+		if (radeon_connector->hpd.hpd != RADEON_HPD_NONE)
+			disable |= 1 << radeon_connector->hpd.hpd;
 	}
 	radeon_irq_kms_disable_hpd(rdev, disable);
 }
drivers/gpu/drm/radeon/rs600.c +4 -2
index 6244f4e44e9a..4b35213fe028 100644
@@ -413,7 +413,8 @@ void rs600_hpd_init(struct radeon_device *rdev)
 		default:
 			break;
 		}
-		enable |= 1 << radeon_connector->hpd.hpd;
+		if (radeon_connector->hpd.hpd != RADEON_HPD_NONE)
+			enable |= 1 << radeon_connector->hpd.hpd;
 		radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd);
 	}
 	radeon_irq_kms_enable_hpd(rdev, enable);
@@ -439,7 +440,8 @@ void rs600_hpd_fini(struct radeon_device *rdev)
 		default:
 			break;
 		}
-		disable |= 1 << radeon_connector->hpd.hpd;
+		if (radeon_connector->hpd.hpd != RADEON_HPD_NONE)
+			disable |= 1 << radeon_connector->hpd.hpd;
 	}
 	radeon_irq_kms_disable_hpd(rdev, disable);
 }
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#11 lore raw

PATCH 11/23 btrfs: fix int32 overflow in shrink_delalloc().

In reply to Ben Hutchings

From: Adam Borowski <[email protected]>

commit 8eb0dfdbda3f56bf7d248ed87fcc383df114ecbb upstream.

UBSAN: Undefined behaviour in fs/btrfs/extent-tree.c:4623:21

signed integer overflow:
10808 * 262144 cannot be represented in type 'int [8]'

If 8192<=items<16384, we request a writeback of an insane number of pages which is benign (everything will be written). But if items>=16384, the space reservation won't be enough.

---
 fs/btrfs/extent-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/btrfs/extent-tree.c +1 -1
index 47cdc6f3390b..a4d7dbe2619e 100644
@@ -4568,7 +4568,7 @@ static void shrink_delalloc(struct btrfs_root *root, u64 to_reclaim, u64 orig,
 
 	/* Calc the number of the pages we need flush for space reservation */
 	items = calc_reclaim_items_nr(root, to_reclaim);
-	to_reclaim = items * EXTENT_SIZE_PER_ITEM;
+	to_reclaim = (u64)items * EXTENT_SIZE_PER_ITEM;
 
 	trans = (struct btrfs_trans_handle *)current->journal_info;
 	block_rsv = &root->fs_info->delalloc_block_rsv;
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#12 lore raw

PATCH 12/23 blk-mq: fix undefined behaviour in order_to_size()

In reply to Ben Hutchings

From: Bartlomiej Zolnierkiewicz <[email protected]>

commit b3a834b1596ac668df206aa2bb1f191c31f5f5e4 upstream.

When this_order variable in blk_mq_init_rq_map() becomes zero the code incorrectly decrements the variable and passes the result to order_to_size() helper causing undefined behaviour:

 UBSAN: Undefined behaviour in block/blk-mq.c:1459:27
 shift exponent 4294967295 is too large for 32-bit type 'unsigned int'
 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc6-00072-g33656a1 #22

Fix the code by checking this_order variable for not having the zero value first.

Fixes 320ae51feed5 ("blk-mq: new multi-queue block IO queueing mechanism")
---
 block/blk-mq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
block/blk-mq.c +1 -1
index c3e461ec40e4..c58fdfdd9ea6 100644
@@ -1494,7 +1494,7 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct blk_mq_tag_set *set,
 		int to_do;
 		void *p;
 
-		while (left < order_to_size(this_order - 1) && this_order)
+		while (this_order && left < order_to_size(this_order - 1))
 			this_order--;
 
 		do {
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#13 lore raw

PATCH 13/23 batman-adv: Fix integer overflow in batadv_iv_ogm_calc_tq

In reply to Ben Hutchings

From: Sven Eckelmann <[email protected]>

commit d285f52cc0f23564fd61976d43fd5b991b4828f6 upstream.

The undefined behavior sanatizer detected an signed integer overflow in a setup with near perfect link quality

    UBSAN: Undefined behaviour in net/batman-adv/bat_iv_ogm.c:1246:25
    signed integer overflow:
    8713350 * 255 cannot be represented in type 'int'

The problems happens because the calculation of mixed unsigned and signed integers resulted in an integer multiplication.

batadv_ogm_packet::tq (u8 255)
* tq_own (u8 255)
* tq_asym_penalty (int 134; max 255)
* tq_iface_penalty (int 255; max 255)

The tq_iface_penalty, tq_asym_penalty and inv_asym_penalty can just be changed to unsigned int because they are not expected to become negative.

Fixes c039876892e3 ("batman-adv: add WiFi penalty")
[bwh: Backported to 4.4: adjust context]
---
 net/batman-adv/bat_iv_ogm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
net/batman-adv/bat_iv_ogm.c +3 -2
index 912d9c36fb1c..3fb6e9e8df7e 100644
@@ -1140,9 +1140,10 @@ static int batadv_iv_ogm_calc_tq(struct batadv_orig_node *orig_node,
 	u8 total_count;
 	u8 orig_eq_count, neigh_rq_count, neigh_rq_inv, tq_own;
 	unsigned int neigh_rq_inv_cube, neigh_rq_max_cube;
-	int tq_asym_penalty, inv_asym_penalty, if_num, ret = 0;
+	int if_num, ret = 0;
+	unsigned int tq_asym_penalty, inv_asym_penalty;
 	unsigned int combined_tq;
-	int tq_iface_penalty;
+	unsigned int tq_iface_penalty;
 
 	/* find corresponding one hop neighbor */
 	rcu_read_lock();
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#14 lore raw

PATCH 14/23 signal: move the "sig < SIGRTMIN" check into siginmask(sig)

In reply to Ben Hutchings

From: Oleg Nesterov <[email protected]>

commit 5c8ccefdf46c5f87d87b694c7fbc04941c2c99a5 upstream.

All the users of siginmask() must ensure that sig < SIGRTMIN. sig_fatal() doesn't and this is wrong:

	UBSAN: Undefined behaviour in kernel/signal.c:911:6
	shift exponent 32 is too large for 32-bit type 'long unsigned int'

the patch doesn't add the neccesary check to sig_fatal(), it moves the check into siginmask() and updates other callers.

---
 include/linux/signal.h | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
include/linux/signal.h +7 -9
index d80259afb9e5..d822bd15dddc 100644
@@ -400,7 +400,9 @@ int unhandled_signal(struct task_struct *tsk, int sig);
 #else
 #define rt_sigmask(sig)	sigmask(sig)
 #endif
-#define siginmask(sig, mask) (rt_sigmask(sig) & (mask))
+
+#define siginmask(sig, mask) \
+	((sig) < SIGRTMIN && (rt_sigmask(sig) & (mask)))
 
 #define SIG_KERNEL_ONLY_MASK (\
 	rt_sigmask(SIGKILL)   |  rt_sigmask(SIGSTOP))
@@ -421,14 +423,10 @@ int unhandled_signal(struct task_struct *tsk, int sig);
         rt_sigmask(SIGCONT)   |  rt_sigmask(SIGCHLD)   | \
 	rt_sigmask(SIGWINCH)  |  rt_sigmask(SIGURG)    )
 
-#define sig_kernel_only(sig) \
-	(((sig) < SIGRTMIN) && siginmask(sig, SIG_KERNEL_ONLY_MASK))
-#define sig_kernel_coredump(sig) \
-	(((sig) < SIGRTMIN) && siginmask(sig, SIG_KERNEL_COREDUMP_MASK))
-#define sig_kernel_ignore(sig) \
-	(((sig) < SIGRTMIN) && siginmask(sig, SIG_KERNEL_IGNORE_MASK))
-#define sig_kernel_stop(sig) \
-	(((sig) < SIGRTMIN) && siginmask(sig, SIG_KERNEL_STOP_MASK))
+#define sig_kernel_only(sig)		siginmask(sig, SIG_KERNEL_ONLY_MASK)
+#define sig_kernel_coredump(sig)	siginmask(sig, SIG_KERNEL_COREDUMP_MASK)
+#define sig_kernel_ignore(sig)		siginmask(sig, SIG_KERNEL_IGNORE_MASK)
+#define sig_kernel_stop(sig)		siginmask(sig, SIG_KERNEL_STOP_MASK)
 
 #define sig_user_defined(t, signr) \
 	(((t)->sighand->action[(signr)-1].sa.sa_handler != SIG_DFL) &&	\
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#15 lore raw

PATCH 15/23 mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()

In reply to Ben Hutchings

From: Seung-Woo Kim <[email protected]>

commit 65257a0deed5aee66b4e3708944f0be62a64cabc upstream.

This patch removes following UBSAN warnings in dw_mci_setup_bus().

  UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14
  shift exponent 250 is too large for 32-bit type 'unsigned int'
  Call trace:
  [<ffffff90080908a8>] dump_backtrace+0x0/0x380
  [<ffffff9008090c3c>] show_stack+0x14/0x20
  [<ffffff90087457b8>] dump_stack+0xe0/0x120
  [<ffffff90087b1360>] ubsan_epilogue+0x18/0x68
  [<ffffff90087b1a94>] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
  [<ffffff9008d89cb8>] dw_mci_setup_bus+0x3a0/0x438
  [...]

  UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27
  shift exponent 250 is too large for 32-bit type 'unsigned int'
  Call trace:
  [<ffffff90080908a8>] dump_backtrace+0x0/0x380
  [<ffffff9008090c3c>] show_stack+0x14/0x20
  [<ffffff90087457b8>] dump_stack+0xe0/0x120
  [<ffffff90087b1360>] ubsan_epilogue+0x18/0x68
  [<ffffff90087b1a94>] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
  [<ffffff9008d89c9c>] dw_mci_setup_bus+0x384/0x438
  [...]

The warnings are caused because of bit shift which is used to filter spamming message for CONFIG_MMC_CLKGATE, but the config is already removed. So this patch just removes the shift.

---
 drivers/mmc/host/dw_mmc.c | 14 +++++---------
 drivers/mmc/host/dw_mmc.h |  4 ----
 2 files changed, 5 insertions(+), 13 deletions(-)
drivers/mmc/host/dw_mmc.c +5 -9
index fb204ee6ff89..0b38014ad315 100644
@@ -1102,12 +1102,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 
 		div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
 
-		if ((clock << div) != slot->__clk_old || force_clkinit)
-			dev_info(&slot->mmc->class_dev,
-				 "Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n",
-				 slot->id, host->bus_hz, clock,
-				 div ? ((host->bus_hz / div) >> 1) :
-				 host->bus_hz, div);
+		dev_info(&slot->mmc->class_dev,
+			 "Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n",
+			 slot->id, host->bus_hz, clock,
+			 div ? ((host->bus_hz / div) >> 1) :
+			 host->bus_hz, div);
 
 		/* disable clock */
 		mci_writel(host, CLKENA, 0);
@@ -1130,9 +1129,6 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
 
 		/* inform CIU */
 		mci_send_cmd(slot, sdmmc_cmd_bits, 0);
-
-		/* keep the clock with reflecting clock dividor */
-		slot->__clk_old = clock << div;
 	}
 
 	host->current_speed = clock;
drivers/mmc/host/dw_mmc.h +0 -4
index f695b58f0613..18fb8f5aaa34 100644
@@ -242,9 +242,6 @@ extern int dw_mci_resume(struct dw_mci *host);
  * @queue_node: List node for placing this node in the @queue list of
  *	&struct dw_mci.
  * @clock: Clock rate configured by set_ios(). Protected by host->lock.
- * @__clk_old: The last updated clock with reflecting clock divider.
- *	Keeping track of this helps us to avoid spamming the console
- *	with CONFIG_MMC_CLKGATE.
  * @flags: Random state bits associated with the slot.
  * @id: Number of this slot.
  * @sdio_id: Number of this slot in the SDIO interrupt registers.
@@ -259,7 +256,6 @@ struct dw_mci_slot {
 	struct list_head	queue_node;
 
 	unsigned int		clock;
-	unsigned int		__clk_old;
 
 	unsigned long		flags;
 #define DW_MMC_CARD_PRESENT	0
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#16 lore raw

PATCH 16/23 UBSAN: fix typo in format string

In reply to Ben Hutchings

From: Nicolas Iooss <[email protected]>

commit 901d805c33fc4c029fc6b2d94ee5fb7d30278045 upstream.

handle_object_size_mismatch() used %pk to format a kernel pointer with pr_err(). This seemed to be a misspelling for %pK, but using this to format a kernel pointer does not make much sence here.

Therefore use %p instead, like in handle_missaligned_access().

---
 lib/ubsan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/ubsan.c +1 -1
index 8799ae5e2e42..fb0409df1bcf 100644
@@ -308,7 +308,7 @@ static void handle_object_size_mismatch(struct type_mismatch_data *data,
 		return;
 
 	ubsan_prologue(&data->location, &flags);
-	pr_err("%s address %pk with insufficient space\n",
+	pr_err("%s address %p with insufficient space\n",
 		type_check_kinds[data->type_check_kind],
 		(void *) ptr);
 	pr_err("for an object of type %s\n", data->type->type_name);
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#17 lore raw

PATCH 17/23 rhashtable: fix shift by 64 when shrinking

In reply to Ben Hutchings

From: Vegard Nossum <[email protected]>

commit 12311959ecf8a3a64676c01b62ce67a0c5f0fd49 upstream.

I got this:

    ================================================================================
    UBSAN: Undefined behaviour in ./include/linux/log2.h:63:13
    shift exponent 64 is too large for 64-bit type 'long unsigned int'
    CPU: 1 PID: 721 Comm: kworker/1:1 Not tainted 4.8.0-rc1+ #87
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
    Workqueue: events rht_deferred_worker
     0000000000000000 ffff88011661f8d8 ffffffff82344f50 0000000041b58ab3
     ffffffff84f98000 ffffffff82344ea4 ffff88011661f900 ffff88011661f8b0
     0000000000000001 ffff88011661f6b8 dffffc0000000000 ffffffff867f7640
    Call Trace:
     [<ffffffff82344f50>] dump_stack+0xac/0xfc
     [<ffffffff82344ea4>] ? _atomic_dec_and_lock+0xc4/0xc4
     [<ffffffff8242f5b8>] ubsan_epilogue+0xd/0x8a
     [<ffffffff82430c41>] __ubsan_handle_shift_out_of_bounds+0x255/0x29a
     [<ffffffff824309ec>] ? __ubsan_handle_out_of_bounds+0x180/0x180
     [<ffffffff84003436>] ? nl80211_req_set_reg+0x256/0x2f0
     [<ffffffff812112ba>] ? print_context_stack+0x8a/0x160
     [<ffffffff81200031>] ? amd_pmu_reset+0x341/0x380
     [<ffffffff823af808>] rht_deferred_worker+0x1618/0x1790
     [<ffffffff823af808>] ? rht_deferred_worker+0x1618/0x1790
     [<ffffffff823ae1f0>] ? rhashtable_jhash2+0x370/0x370
     [<ffffffff8134c12d>] ? process_one_work+0x6fd/0x1970
     [<ffffffff8134c1cf>] process_one_work+0x79f/0x1970
     [<ffffffff8134c12d>] ? process_one_work+0x6fd/0x1970
     [<ffffffff8134ba30>] ? try_to_grab_pending+0x4c0/0x4c0
     [<ffffffff8134d564>] ? worker_thread+0x1c4/0x1340
     [<ffffffff8134d8ff>] worker_thread+0x55f/0x1340
     [<ffffffff845e904f>] ? __schedule+0x4df/0x1d40
     [<ffffffff8134d3a0>] ? process_one_work+0x1970/0x1970
     [<ffffffff8134d3a0>] ? process_one_work+0x1970/0x1970
     [<ffffffff813642f7>] kthread+0x237/0x390
     [<ffffffff813640c0>] ? __kthread_parkme+0x280/0x280
     [<ffffffff845f8c93>] ? _raw_spin_unlock_irq+0x33/0x50
     [<ffffffff845f95df>] ret_from_fork+0x1f/0x40
     [<ffffffff813640c0>] ? __kthread_parkme+0x280/0x280
    ================================================================================

roundup_pow_of_two() is undefined when called with an argument of 0, so let's avoid the call and just fall back to ht->p.min_size (which should never be smaller than HASH_MIN_SIZE).

---
 lib/rhashtable.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
lib/rhashtable.c +4 -2
index 51282f579760..d7f5b4d035ff 100644
@@ -324,12 +324,14 @@ static int rhashtable_expand(struct rhashtable *ht)
 static int rhashtable_shrink(struct rhashtable *ht)
 {
 	struct bucket_table *new_tbl, *old_tbl = rht_dereference(ht->tbl, ht);
-	unsigned int size;
+	unsigned int nelems = atomic_read(&ht->nelems);
+	unsigned int size = 0;
 	int err;
 
 	ASSERT_RHT_MUTEX(ht);
 
-	size = roundup_pow_of_two(atomic_read(&ht->nelems) * 3 / 2);
+	if (nelems)
+		size = roundup_pow_of_two(nelems * 3 / 2);
 	if (size < ht->p.min_size)
 		size = ht->p.min_size;
 
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#18 lore raw

PATCH 18/23 time: Avoid undefined behaviour in ktime_add_safe()

In reply to Ben Hutchings

From: Vegard Nossum <[email protected]>

commit 979515c5645830465739254abc1b1648ada41518 upstream.

I ran into this:

    ================================================================================
    UBSAN: Undefined behaviour in kernel/time/hrtimer.c:310:16
    signed integer overflow:
    9223372036854775807 + 50000 cannot be represented in type 'long long int'
    CPU: 2 PID: 4798 Comm: trinity-c2 Not tainted 4.8.0-rc1+ #91
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
     0000000000000000 ffff88010ce6fb88 ffffffff82344740 0000000041b58ab3
     ffffffff84f97a20 ffffffff82344694 ffff88010ce6fbb0 ffff88010ce6fb60
     000000000000c350 ffff88010ce6f968 dffffc0000000000 ffffffff857bc320
    Call Trace:
     [<ffffffff82344740>] dump_stack+0xac/0xfc
     [<ffffffff82344694>] ? _atomic_dec_and_lock+0xc4/0xc4
     [<ffffffff8242df78>] ubsan_epilogue+0xd/0x8a
     [<ffffffff8242e6b4>] handle_overflow+0x202/0x23d
     [<ffffffff8242e4b2>] ? val_to_string.constprop.6+0x11e/0x11e
     [<ffffffff8236df71>] ? timerqueue_add+0x151/0x410
     [<ffffffff81485c48>] ? hrtimer_start_range_ns+0x3b8/0x1380
     [<ffffffff81795631>] ? memset+0x31/0x40
     [<ffffffff8242e6fd>] __ubsan_handle_add_overflow+0xe/0x10
     [<ffffffff81488ac9>] hrtimer_nanosleep+0x5d9/0x790
     [<ffffffff814884f0>] ? hrtimer_init_sleeper+0x80/0x80
     [<ffffffff813a9ffb>] ? __might_sleep+0x5b/0x260
     [<ffffffff8148be10>] common_nsleep+0x20/0x30
     [<ffffffff814906c7>] SyS_clock_nanosleep+0x197/0x210
     [<ffffffff81490530>] ? SyS_clock_getres+0x150/0x150
     [<ffffffff823c7113>] ? __this_cpu_preempt_check+0x13/0x20
     [<ffffffff8162ef60>] ? __context_tracking_exit.part.3+0x30/0x1b0
     [<ffffffff81490530>] ? SyS_clock_getres+0x150/0x150
     [<ffffffff81007bd3>] do_syscall_64+0x1b3/0x4b0
     [<ffffffff845f85aa>] entry_SYSCALL64_slow_path+0x25/0x25
    ================================================================================

Add a new ktime_add_unsafe() helper which doesn't check for overflow, but doesn't throw a UBSAN warning when it does overflow either.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Richard Cochran <[email protected]>
Cc Prarit Bhargava <[email protected]>
---
 include/linux/ktime.h | 7 +++++++
 kernel/time/hrtimer.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)
include/linux/ktime.h +7 -0
index 2b6a204bd8d4..3ffc69ebe967 100644
@@ -64,6 +64,13 @@ static inline ktime_t ktime_set(const s64 secs, const unsigned long nsecs)
 		({ (ktime_t){ .tv64 = (lhs).tv64 + (rhs).tv64 }; })
 
 /*
+ * Same as ktime_add(), but avoids undefined behaviour on overflow; however,
+ * this means that you must check the result for overflow yourself.
+ */
+#define ktime_add_unsafe(lhs, rhs) \
+		({ (ktime_t){ .tv64 = (u64) (lhs).tv64 + (rhs).tv64 }; })
+
+/*
  * Add a ktime_t variable and a scalar nanosecond value.
  * res = kt + nsval:
  */
kernel/time/hrtimer.c +1 -1
index 17f7bcff1e02..1dc94768b5a3 100644
@@ -312,7 +312,7 @@ EXPORT_SYMBOL_GPL(__ktime_divns);
  */
 ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs)
 {
-	ktime_t res = ktime_add(lhs, rhs);
+	ktime_t res = ktime_add_unsafe(lhs, rhs);
 
 	/*
 	 * We use KTIME_SEC_MAX here, the maximum timeout which we can
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#19 lore raw

PATCH 19/23 pwm: samsung: Fix to use lowest div for large enough modulation bits

In reply to Ben Hutchings

From: Seung-Woo Kim <[email protected]>

commit 04d68dea26b0a409d44e87ea573a131b6dc67e78 upstream.

Quoted 1 line

From pwm_samsung_calc_tin(), there is routine to find the lowest divider

possible to generate lower frequency than requested one. But it is always possible to generate requested frequency with large enough modulation bits except on s3c24xx, so this patch fixes to use lowest div for the case. This patch removes following UBSAN warning:

   UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
   shift exponent 32 is too large for 32-bit type 'long unsigned int'
   [...]
   [<c0670248>] (ubsan_epilogue) from [<c06707b4>] (__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
   [<c06707b4>] (__ubsan_handle_shift_out_of_bounds) from [<c0694b28>] (pwm_samsung_config+0x508/0x6a4)
   [<c0694b28>] (pwm_samsung_config) from [<c069286c>] (pwm_apply_state+0x174/0x40c)
   [<c069286c>] (pwm_apply_state) from [<c0b2e070>] (pwm_fan_probe+0xc8/0x488)
   [<c0b2e070>] (pwm_fan_probe) from [<c07ba8b0>] (platform_drv_probe+0x70/0x150)
   [...]
---
 drivers/pwm/pwm-samsung.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/pwm/pwm-samsung.c +12 -3
index ada2d326dc3e..f113cda47032 100644
@@ -193,9 +193,18 @@ static unsigned long pwm_samsung_calc_tin(struct samsung_pwm_chip *chip,
 	 * divider settings and choose the lowest divisor that can generate
 	 * frequencies lower than requested.
 	 */
-	for (div = variant->div_base; div < 4; ++div)
-		if ((rate >> (variant->bits + div)) < freq)
-			break;
+	if (variant->bits < 32) {
+		/* Only for s3c24xx */
+		for (div = variant->div_base; div < 4; ++div)
+			if ((rate >> (variant->bits + div)) < freq)
+				break;
+	} else {
+		/*
+		 * Other variants have enough counter bits to generate any
+		 * requested rate, so no need to check higher divisors.
+		 */
+		div = variant->div_base;
+	}
 
 	pwm_samsung_set_divisor(chip, chan, BIT(div));
 
Signature · 6 lines
2.10.2


-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#20 lore raw

PATCH 20/23 drm: fix signed integer overflow

In reply to Ben Hutchings

From: Xie XiuQi <[email protected]>

commit ae0119f5f73b1e9cf7177fbbeea68d74c5751def upstream.

Use 1UL for unsigned long, or we'll meet a overflow issue with UBSAN.

[   15.589489] UBSAN: Undefined behaviour in drivers/gpu/drm/drm_hashtab.c:145:35
[   15.589500] signed integer overflow:
[   15.589999] -2147483648 - 1 cannot be represented in type 'int'
[   15.590434] CPU: 2 PID: 294 Comm: plymouthd Not tainted 3.10.0-327.28.3.el7.x86_64 #1
[   15.590653] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 01/07/2011
[   15.591001]  1ffff1000670fe83 000000000d6b385e ffff88003387f3e0 ffffffff81ee3140
[   15.591028]  ffff88003387f3f8 ffffffff81ee31fd ffffffffa032f460 ffff88003387f560
[   15.591044]  ffffffff81ee46e2 0000002d00000009 0000000000000001 0000000041b58ab3
[   15.591059] Call Trace:
[   15.591078]  [<ffffffff81ee3140>] dump_stack+0x1e/0x20
[   15.591093]  [<ffffffff81ee31fd>] ubsan_epilogue+0x12/0x55
[   15.591109]  [<ffffffff81ee46e2>] handle_overflow+0x1ba/0x215
[   15.591126]  [<ffffffff81ee4528>] ? __ubsan_handle_negate_overflow+0x162/0x162
[   15.591146]  [<ffffffff8103416c>] ? print_context_stack+0x9c/0x160
[   15.591163]  [<ffffffff81031df2>] ? dump_trace+0x252/0x750
[   15.591181]  [<ffffffff81739023>] ? __list_add+0x93/0x160
[   15.591197]  [<ffffffff81ee4798>] __ubsan_handle_sub_overflow+0x2a/0x31
[   15.591261]  [<ffffffffa0282140>] drm_ht_just_insert_please+0x1e0/0x200 [drm]
[   15.591290]  [<ffffffffa0528c7a>] ttm_base_object_init+0x10a/0x270 [ttm]
[   15.591316]  [<ffffffffa052a34c>] ttm_vt_lock+0x28c/0x3a0 [ttm]
[   15.591343]  [<ffffffffa052a0c0>] ? ttm_write_lock+0x180/0x180 [ttm]
[   15.591362]  [<ffffffff81419526>] ? kasan_unpoison_shadow+0x36/0x50
[   15.591379]  [<ffffffff81419526>] ? kasan_unpoison_shadow+0x36/0x50
[   15.591396]  [<ffffffff81419526>] ? kasan_unpoison_shadow+0x36/0x50
[   15.591413]  [<ffffffff81419526>] ? kasan_unpoison_shadow+0x36/0x50
[   15.591442]  [<ffffffffa061cbe1>] vmw_master_set+0x121/0x470 [vmwgfx]
[   15.591459]  [<ffffffff811773a5>] ? __init_waitqueue_head+0x45/0x70
[   15.591487]  [<ffffffffa061cac0>] ? vmw_master_drop+0x310/0x310 [vmwgfx]
[   15.591535]  [<ffffffffa026946a>] drm_open+0x92a/0xc00 [drm]
[   15.591563]  [<ffffffffa0619ff0>] ? vmw_driver_open+0x170/0x170 [vmwgfx]
[   15.591610]  [<ffffffffa0268b40>] ? drm_poll+0xe0/0xe0 [drm]
[   15.591661]  [<ffffffffa02797b4>] drm_stub_open+0x224/0x330 [drm]
[   15.591711]  [<ffffffffa0279590>] ? drm_minor_acquire+0x240/0x240 [drm]
[   15.591727]  [<ffffffff8145fa8a>] chrdev_open+0x1fa/0x3f0
[   15.591742]  [<ffffffff8145f890>] ? cdev_put+0x50/0x50
[   15.591761]  [<ffffffff814f6dc3>] ? __fsnotify_parent+0x53/0x210
[   15.591778]  [<ffffffff8144fde1>] do_dentry_open+0x351/0x670
[   15.591792]  [<ffffffff8145f890>] ? cdev_put+0x50/0x50
[   15.591807]  [<ffffffff814503c2>] vfs_open+0xa2/0x170
[   15.591824]  [<ffffffff8147b5df>] do_last+0xccf/0x2c80
[   15.591842]  [<ffffffff8147a910>] ? filename_create+0x320/0x320
[   15.591858]  [<ffffffff81472549>] ? path_init+0x1b9/0xa90
[   15.591875]  [<ffffffff81472390>] ? mountpoint_last+0x9a0/0x9a0
[   15.591894]  [<ffffffff815f9ccf>] ? selinux_file_alloc_security+0xcf/0x130
[   15.591911]  [<ffffffff8147d777>] path_openat+0x1e7/0xcc0
[   15.591927]  [<ffffffff81031df2>] ? dump_trace+0x252/0x750
[   15.591943]  [<ffffffff8147d590>] ? do_last+0x2c80/0x2c80
[   15.591959]  [<ffffffff81739023>] ? __list_add+0x93/0x160
[   15.591974]  [<ffffffff8104b48d>] ? save_stack_trace+0x7d/0xb0
[   15.591989]  [<ffffffff81480824>] do_filp_open+0xa4/0x160
[   15.592004]  [<ffffffff81480780>] ? user_path_mountpoint_at+0x50/0x50
[   15.592022]  [<ffffffff8149d755>] ? __alloc_fd+0x175/0x300
[   15.592039]  [<ffffffff81453127>] do_sys_open+0x1b7/0x3f0
[   15.592054]  [<ffffffff81452f70>] ? filp_open+0x80/0x80
[   15.592070]  [<ffffffff81453392>] SyS_open+0x32/0x40
[   15.592088]  [<ffffffff81f08989>] system_call_fastpath+0x16/0x1b
---
 drivers/gpu/drm/drm_hashtab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/drm_hashtab.c +1 -1
index c3b80fd65d62..fbbd9f0c2eef 100644
@@ -142,7 +142,7 @@ int drm_ht_just_insert_please(struct drm_open_hash *ht, struct drm_hash_item *it
 			      unsigned long add)
 {
 	int ret;
-	unsigned long mask = (1 << bits) - 1;
+	unsigned long mask = (1UL << bits) - 1;
 	unsigned long first, unshifted_key;
 
 	unshifted_key = hash_long(seed, bits);
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#21 lore raw

PATCH 21/23 xfs: fix signed integer overflow

In reply to Ben Hutchings

From: Xie XiuQi <[email protected]>

commit 79c350e45ebc5a718cc2d7114b45ad560069423d upstream.

Use 1U for unsigned int to avoid a overflow warning from UBSAN.

[   31.910858] UBSAN: Undefined behaviour in fs/xfs/xfs_buf_item.c:889:25
[   31.911252] signed integer overflow:
[   31.911478] -2147483648 - 1 cannot be represented in type 'int'
[   31.911846] CPU: 1 PID: 1011 Comm: tuned Tainted: G    B          ---- -------   3.10.0-327.28.3.el7.x86_64 #1
[   31.911857] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 01/07/2011
[   31.911866]  1ffff1004069cd3b 0000000076bec3fd ffff8802034e69a0 ffffffff81ee3140
[   31.911883]  ffff8802034e69b8 ffffffff81ee31fd ffffffffa0ad79e0 ffff8802034e6b20
[   31.911898]  ffffffff81ee46e2 0000002d515470c0 0000000000000001 0000000041b58ab3
[   31.911913] Call Trace:
[   31.911932]  [<ffffffff81ee3140>] dump_stack+0x1e/0x20
[   31.911947]  [<ffffffff81ee31fd>] ubsan_epilogue+0x12/0x55
[   31.911964]  [<ffffffff81ee46e2>] handle_overflow+0x1ba/0x215
[   31.912083]  [<ffffffff81ee4798>] __ubsan_handle_sub_overflow+0x2a/0x31
[   31.912204]  [<ffffffffa08676fb>] xfs_buf_item_log+0x34b/0x3f0 [xfs]
[   31.912314]  [<ffffffffa0880490>] xfs_trans_log_buf+0x120/0x260 [xfs]
[   31.912402]  [<ffffffffa079a890>] xfs_btree_log_recs+0x80/0xc0 [xfs]
[   31.912490]  [<ffffffffa07a29f8>] xfs_btree_delrec+0x11a8/0x2d50 [xfs]
[   31.913589]  [<ffffffffa07a86f9>] xfs_btree_delete+0xc9/0x260 [xfs]
[   31.913762]  [<ffffffffa075b5cf>] xfs_free_ag_extent+0x63f/0xe20 [xfs]
[   31.914339]  [<ffffffffa075ec0f>] xfs_free_extent+0x2af/0x3e0 [xfs]
[   31.914641]  [<ffffffffa0801b2b>] xfs_bmap_finish+0x32b/0x4b0 [xfs]
[   31.914841]  [<ffffffffa083c2e7>] xfs_itruncate_extents+0x3b7/0x740 [xfs]
[   31.915216]  [<ffffffffa08342fa>] xfs_setattr_size+0x60a/0x860 [xfs]
[   31.915471]  [<ffffffffa08345ea>] xfs_vn_setattr+0x9a/0xe0 [xfs]
[   31.915590]  [<ffffffff8149ad38>] notify_change+0x5c8/0x8a0
[   31.915607]  [<ffffffff81450f22>] do_truncate+0x122/0x1d0
[   31.915640]  [<ffffffff8147beee>] do_last+0x15de/0x2c80
[   31.915707]  [<ffffffff8147d777>] path_openat+0x1e7/0xcc0
[   31.915802]  [<ffffffff81480824>] do_filp_open+0xa4/0x160
[   31.915848]  [<ffffffff81453127>] do_sys_open+0x1b7/0x3f0
[   31.915879]  [<ffffffff81453392>] SyS_open+0x32/0x40
[   31.915897]  [<ffffffff81f08989>] system_call_fastpath+0x16/0x1b
[  240.086809] UBSAN: Undefined behaviour in fs/xfs/xfs_buf_item.c:866:34
[  240.086820] signed integer overflow:
[  240.086830] -2147483648 - 1 cannot be represented in type 'int'
[  240.086846] CPU: 1 PID: 12969 Comm: rm Tainted: G    B          ---- -------   3.10.0-327.28.3.el7.x86_64 #1
[  240.086857] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 01/07/2011
[  240.086868]  1ffff10040491def 00000000e2ea59c1 ffff88020248ef40 ffffffff81ee3140
[  240.086885]  ffff88020248ef58 ffffffff81ee31fd ffffffffa0ad79e0 ffff88020248f0c0
[  240.086901]  ffffffff81ee46e2 0000002d02488000 0000000000000001 0000000041b58ab3
[  240.086915] Call Trace:
[  240.086938]  [<ffffffff81ee3140>] dump_stack+0x1e/0x20
[  240.086953]  [<ffffffff81ee31fd>] ubsan_epilogue+0x12/0x55
[  240.086971]  [<ffffffff81ee46e2>] handle_overflow+0x1ba/0x215

...

---
 fs/xfs/xfs_buf_item.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
fs/xfs/xfs_buf_item.c +2 -2
index 7e986da34f6c..04b1d96e49c1 100644
@@ -865,7 +865,7 @@ xfs_buf_item_log_segment(
 	 */
 	if (bit) {
 		end_bit = MIN(bit + bits_to_set, (uint)NBWORD);
-		mask = ((1 << (end_bit - bit)) - 1) << bit;
+		mask = ((1U << (end_bit - bit)) - 1) << bit;
 		*wordp |= mask;
 		wordp++;
 		bits_set = end_bit - bit;
@@ -888,7 +888,7 @@ xfs_buf_item_log_segment(
 	 */
 	end_bit = bits_to_set - bits_set;
 	if (end_bit) {
-		mask = (1 << end_bit) - 1;
+		mask = (1U << end_bit) - 1;
 		*wordp |= mask;
 	}
 }
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#22 lore raw

PATCH 22/23 net: get rid of an signed integer overflow in ip_idents_reserve()

In reply to Ben Hutchings

From: Eric Dumazet <[email protected]>

commit adb03115f4590baa280ddc440a8eff08a6be0cb7 upstream.

Jiri Pirko reported an UBSAN warning happening in ip_idents_reserve()

[] UBSAN: Undefined behaviour in ./arch/x86/include/asm/atomic.h:156:11 [] signed integer overflow:
[] -2117905507 + -695755206 cannot be represented in type 'int'

Since we do not have uatomic_add_return() yet, use atomic_cmpxchg() so that the arithmetics can be done using unsigned int.

Fixes 04ca6973f7c1 ("ip: make IP identifiers less predictable")
---
 net/ipv4/route.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
net/ipv4/route.c +8 -2
index b050cf980a57..a0270af75fa5 100644
@@ -476,12 +476,18 @@ u32 ip_idents_reserve(u32 hash, int segs)
 	atomic_t *p_id = ip_idents + hash % IP_IDENTS_SZ;
 	u32 old = ACCESS_ONCE(*p_tstamp);
 	u32 now = (u32)jiffies;
-	u32 delta = 0;
+	u32 new, delta = 0;
 
 	if (old != now && cmpxchg(p_tstamp, old, now) == old)
 		delta = prandom_u32_max(now - old);
 
-	return atomic_add_return(segs + delta, p_id) - segs;
+	/* Do not use atomic_add_return() as it makes UBSAN unhappy */
+	do {
+		old = (u32)atomic_read(p_id);
+		new = old + delta + segs;
+	} while (atomic_cmpxchg(p_id, old, new) != old);
+
+	return new - segs;
 }
 EXPORT_SYMBOL(ip_idents_reserve);
 
Signature · 7 lines
2.10.2



-- 
Ben Hutchings
Software Developer, Codethink Ltd.
#23 lore raw

PATCH 23/23 mlx4: remove unused fields

In reply to Ben Hutchings

From: David Decotigny <[email protected]>

commit 5038056e6bd45788235e97e3bcfc43f96c52ca84 upstream.

This also can address following UBSAN warnings:

[   36.640343] ================================================================================
[   36.648772] UBSAN: Undefined behaviour in drivers/net/ethernet/mellanox/mlx4/fw.c:857:26
[   36.656853] shift exponent 64 is too large for 32-bit type 'int'
[   36.663348] ================================================================================
[   36.671783] ================================================================================
[   36.680213] UBSAN: Undefined behaviour in drivers/net/ethernet/mellanox/mlx4/fw.c:861:27
[   36.688297] shift exponent 35 is too large for 32-bit type 'int'
[   36.694702] ================================================================================

Tested:
reboot with UBSAN, no warning.

---
 drivers/net/ethernet/mellanox/mlx4/fw.c | 4 ----
 drivers/net/ethernet/mellanox/mlx4/fw.h | 2 --
 2 files changed, 6 deletions(-)
drivers/net/ethernet/mellanox/mlx4/fw.c +0 -4
index 90db94e83fde..ebff502db8ff 100644
@@ -760,12 +760,8 @@ int mlx4_QUERY_DEV_CAP(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
 	dev_cap->max_eqs = 1 << (field & 0xf);
 	MLX4_GET(field, outbox, QUERY_DEV_CAP_RSVD_MTT_OFFSET);
 	dev_cap->reserved_mtts = 1 << (field >> 4);
-	MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_MRW_SZ_OFFSET);
-	dev_cap->max_mrw_sz = 1 << field;
 	MLX4_GET(field, outbox, QUERY_DEV_CAP_RSVD_MRW_OFFSET);
 	dev_cap->reserved_mrws = 1 << (field & 0xf);
-	MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_MTT_SEG_OFFSET);
-	dev_cap->max_mtt_seg = 1 << (field & 0x3f);
 	MLX4_GET(size, outbox, QUERY_DEV_CAP_NUM_SYS_EQ_OFFSET);
 	dev_cap->num_sys_eqs = size & 0xfff;
 	MLX4_GET(field, outbox, QUERY_DEV_CAP_MAX_REQ_QP_OFFSET);
drivers/net/ethernet/mellanox/mlx4/fw.h +0 -2
index 08de5555c2f4..711af4e506f3 100644
@@ -78,9 +78,7 @@ struct mlx4_dev_cap {
 	int max_eqs;
 	int num_sys_eqs;
 	int reserved_mtts;
-	int max_mrw_sz;
 	int reserved_mrws;
-	int max_mtt_seg;
 	int max_requester_per_qp;
 	int max_responder_per_qp;
 	int max_rdma_global;
Signature · 6 lines
2.10.2


-- 
Ben Hutchings
Software Developer, Codethink Ltd.