Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update __usbnet_{read|write}_cmd #79

Closed
wants to merge 83 commits into from
Closed

Update __usbnet_{read|write}_cmd #79

wants to merge 83 commits into from

Conversation

thazhemadam
Copy link
Contributor

Often, the issue is __usbnet_{read|write}_cmd doesn't read/write fully, and unitialized data may be passed along in some cases.
However, Greg recently created a new API (which Oliver updated a little) that allows only complete reads/writes to address this; and I believe this might just fix a bunch of issues.

…ffer console"

This reverts commit 5854059.

KMSAN starts reporting a lot of false positives if the
drm_fb_helper_dirty_work() callback is invoked. Perhaps the vmap dark
magic in drivers/gpu/drm/drm_gem_vram_helper.c is involved.
Disable the shadow buffer for now.

Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-mm@kvack.org

---

Change-Id: I255645030b160bc3fe9cf0a472fc78cd4144545b
…DME.md

Move along, nothing to see here.
This patch is only to reduce diff between KMSAN development and review
branches.

Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-mm@kvack.org

---

Change-Id: I415f1c33906f09d84b7ae0f4ebac1d4a70bd0648
Unpoisoning area->list seems to fix all the problems with kcov_remote_start()
false positives.
This depends on the "kcov: remote coverage support" patch by Andrey
Konovalov.

Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-mm@kvack.org

---

Change-Id: I8852891d8bf39a272f2e6305a209d59058c63512
…o the network

This patch might be working, but we don't send it for upstream review to reduce
patchset size.

Calling kmsan_check_skb() lets KMSAN check the bytes to be transferred
over the network for being initialized.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

This patch was previously called "kmsan: call KMSAN hooks where needed"

v4:
 - split this patch away

Change-Id: Iff48409dc50341d59e355ce3ec11d4722f0799e2
Some users (currently only KMSAN) may want to use spare bits in
depot_stack_handle_t. Let them do so and provide get_dsh_extra_bits()
and set_dsh_extra_bits() to access those bits.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
---

Change-Id: I23580dbde85908eeda0bdd8f83a8c3882ab3e012
Add Documentation/dev-tools/kmsan.rst and reference it in the dev-tools
index.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---
v4:
 - address comments by Marco Elver:
  - remove contractions
  - fix references
  - minor fixes

v6:
 - address comments by Andrey Konovalov:
  - add a link to 'Metadata allocation'
  - rewrite the paragraph about ignored pages
  - fix empty lines after headings
  - minor changes
 - remove mentioning of  __GFP_NO_KMSAN_SHADOW

Change-Id: Iac6345065e6804ef811f1124fdf779c67ff1530e
__no_sanitize_memory is a function attribute that makes KMSAN
ignore the uninitialized values coming from the function's
inputs, and initialize the function's outputs.

Functions marked with this attribute can't be inlined into functions
not marked with it, and vice versa.

__SANITIZE_MEMORY__ is a macro that's defined iff the file is
instrumented with KMSAN. This is not the same as CONFIG_KMSAN, which is
defined for every file.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
Acked-by: Marco Elver <elver@google.com>
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>

---

v4:
 - dropped an unnecessary comment as requested by Marco Elver

Change-Id: I1f1672652c8392f15f7ca8ac26cd4e71f9cc1e4b
KMSAN is going to use 3/4 of existing vmalloc space to hold the
metadata, therefore we lower VMALLOC_END to make sure vmalloc() doesn't
allocate past the first 1/4.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

v6:
 - addressed comments by Andrey Konovalov

Change-Id: Iaa5e8e0fc2aa66c956f937f5a1de6e5ef40d57cc
This patch adds the core parts of KMSAN runtime and associated files:

  - include/linux/kmsan-checks.h: user API to poison/unpoison/check memory
  - include/linux/kmsan.h: declarations of KMSAN memory hooks to be
    referenced outside KMSAN runtime
  - lib/kmsan_annotations.c: non-inlineable implementation of KMSAN_INIT()
  - lib/Kconfig.kmsan: declarations for CONFIG_KMSAN and CONFIG_TEST_KMSAN
  - mm/kmsan/Makefile: boilerplate Makefile
  - mm/kmsan/kmsan.h: internal KMSAN declarations
  - mm/kmsan/kmsan.c: core functions that operate with shadow and
    origin memory and perform checks, utility functions
  - mm/kmsan/kmsan_shadow.c: routines that encapsulate metadata creation
    and addressing
  - mm/kmsan/kmsan_init.c: KMSAN initialization routines
  - scripts/Makefile.kmsan: CFLAGS_KMSAN

The patch also adds the necessary bookkeeping bits to struct page and
struct task_struct:
 - each struct page now contains pointers to two struct pages holding
   KMSAN metadata (shadow and origins) for the original struct page;
 - each task_struct contains a struct kmsan_task_state used to track
   the metadata of function parameters and return values for that task.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---
v2:
 - dropped kmsan_handle_vprintk()
 - use locking for single kmsan_pr_err() calls
 - don't try to understand we're inside printk()
v3:
 - fix an endless loop in __msan_poison_alloca()
 - implement kmsan_handle_dma()
 - dropped kmsan_handle_i2c_transfer()
 - fixed compilation with UNWINDER_ORC
 - dropped assembly hooks for system calls
v4:
 - splitted away some runtime parts to ease the review process
 - fix a lot of comments by Marco Elver and Andrey Konovalov:
 -- clean up headers and #defines, remove debugging code
 -- dropped kmsan_pr_* macros, fixed reporting code
 -- removed TODOs
 -- simplified kmsan_get_shadow_origin_ptr()
 - actually filter out IRQ frames using filter_irq_stacks()
 - simplify kmsan_get_metadata()
 - include build_bug.h into kmsan-checks.h
 - don't instrument KMSAN files with stackprotector
 - squashed "kmsan: add KMSAN bits to struct page and struct
   task_struct" into this patch as requested by Marco Elver
v5:
 - s/kmsan_softirq/kmsan_context everywhere (spotted by kbuild test
   robot <lkp@intel.com>)
v6:
 - moved functions used by KMSAN_INIT_VAL to a separate file
 - change prototype of kmsan_handle_dma(), add kmsan_handle_dma_sg()
 - implemented eager metadata allocation: most shadow/origin pages are
   allocated when releasing memblock memory at boot time.
 - dropped kmsan_split_page()
 - added KMSAN_ prefix to vmalloc range defines

kmsan: add KMSAN_ prefix
kmsan_instr.c contains the functions called by KMSAN instrumentation.
These include functions that:
 - return shadow/origin pointers for memory accesses;
 - poison and unpoison local variables;
 - provide KMSAN context state to pass metadata for function arguments;
 - perform string operations (mem*) on metadata;
 - tell KMSAN to report an error.

This patch has been split away from the rest of KMSAN runtime to
simplify the review process.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

v4:
 - split this patch away as requested by Andrey Konovalov
 - removed redundant address checks when copying shadow
 - fix __msan_memmove prototype

Change-Id: I826272ed2ebe8ab8ef61a9d4cccdcf07c7b6b499
This patch provides hooks that subsystems use to notify KMSAN about
changes in the kernel state. Such changes include:
 - page operations (allocation, deletion, splitting, mapping);
 - memory allocation and deallocation;
 - entering and leaving IRQ/NMI/softirq contexts;
 - copying data between kernel, userspace and hardware.

This patch has been split away from the rest of KMSAN runtime to
simplify the review process.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

v4:
 - fix a lot of comments by Marco Elver and Andrey Konovalov:
 - clean up headers and #defines, remove debugging code
 - simplified KMSAN entry hooks
 - fixed kmsan_check_skb()

v6:
 - change prototype of kmsan_handle_dma(), add kmsan_handle_dma_sg()
 - check for !PageHighMem before calling page_address()
 - remove __GFP_NO_KMSAN_SHADOW

Change-Id: I99d1f34f26bef122897cb840dac8d5b34d2b6a80
READ_ONCE_NOCHECK() is already used by KASAN to ignore memory accesses
from e.g. stack unwinders.
Define READ_ONCE_NOCHECK() for KMSAN so that it returns initialized
values. This helps defeat false positives from leftover stack contents.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
---
v3:
 - removed unnecessary #ifdef as requested by Mark Rutland
v4:
 - added an #include as requested by Marco Elver
v6:
 - READ_ONCE_NOCHECK() moved to include/asm-generic/rwonce.h

Change-Id: Ib38369ba038ab3b581d8e45b81036c3304fb79cb
To avoid false positives, assume that reading from the task stack
always produces initialized values.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
Acked-by: Marco Elver <elver@google.com>

---
v4:
 - added an #include as requested by Marco Elver

Change-Id: Ie73e5a41fdc8195699928e65f5cbe0d3d3c9e2fa
KMSAN assumes shadow and origin pages for every allocated page are
accessible. For pages in vmalloc region those metadata pages reside in
[VMALLOC_END, VMALLOC_META_END), therefore we must sync a bigger memory
region.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

v6:
 - added KMSAN_ prefixes to metadata range defines
 - TODO

Change-Id: I0d54855489870ef1180b37fe2120b601da464bf7

arch: x86: add KMSAN_ prefix
The initial commit adds several tests that trigger KMSAN warnings in
simple cases.
To use, build the kernel with CONFIG_TEST_KMSAN and do
`insmod test_kmsan.ko`

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
---
v2:
 - added printk_test()
v4:
 - test_kmsan: don't report -Wuninitialized warnings in the test
 - test_kmsan.c: addressed comments by Andrey Konovalov

Change-Id: I287e86ae83a82b770f2baa46e5bbdce1dfa65195
KMSAN is unable to understand when initialized values come from assembly.
Disable accelerated configs in KMSAN builds to prevent false positive
reports.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Biggers <ebiggers@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

v4:
 - shorten comments as requested by Marco Elver

v5:
 - move the 'depends' directives together, added missing configs as
   requested by Eric Biggers

Change-Id: Iddc71a2a27360e036d719c0940ebf15553cf8de8
KMSAN doesn't currently support UNWINDER_ORC, causing the kernel to
freeze at boot time.
See http://github.com/google/kmsan/issues/48.

Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---
This patch is part of "kmsan: Kconfig changes to disable options
incompatible with KMSAN", which was split into smaller pieces.

Change-Id: I9cb6ebbaeb9a38e9e1d015c68ab77d40420a7ad0
Instrumenting some files with KMSAN will result in kernel being unable
to link, boot or crashing at runtime for various reasons (e.g. infinite
recursion caused by instrumentation hooks calling instrumented code again).

Disable KMSAN in the following places:
 - arch/x86/boot and arch/x86/realmode/rm, as KMSAN doesn't work for i386;
 - arch/x86/entry/vdso, which isn't linked with KMSAN runtime;
 - three files in arch/x86/kernel - boot problems;
 - arch/x86/mm/cpu_entry_area.c - recursion;
 - EFI stub - build failures;
 - kcov, stackdepot, lockdep - recursion.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>

---

v4:
 - fix lockdep support by not instrumenting lockdep.c
 - unified comments with KCSAN

v6:
 - rebase after KCSAN merge

Change-Id: I90961eabf2dcb9ae992aed259088953bad5e4d6d
In order to report uninitialized memory coming from heap allocations
KMSAN has to poison them unless they're created with __GFP_ZERO.

It's handy that we need KMSAN hooks in the places where
init_on_alloc/init_on_free initialization is performed.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
---
v3:
 - reverted unrelated whitespace changes

Change-Id: I51103b7981d3aabed747d0c85cbdc85568665871
Depending on the value of is_out kmsan_handle_urb() KMSAN either
marks the data copied to the kernel from a USB device as initialized,
or checks the data sent to the device for being initialized.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

This patch was previously called "kmsan: call KMSAN hooks where needed"

v4:
 - split this patch away

Change-Id: Idd0f8ce858975112285706ffb7286f570bd3007b
Tell KMSAN that a new task is created, so the tool creates a backing
metadata structure for that task.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

This patch was previously called "kmsan: call KMSAN hooks where needed"

v4:
 - split this patch away

Change-Id: I7a6a83419b0e038f8993175461255f462a430205
In vprintk_store(), vscnprintf() may return an uninitialized text_len
value if any of its arguments are uninitialized. In that case KMSAN will
report one or more errors in vscnprintf() itself, but it doesn't make
much sense to track that value further, as it may trigger more errors in
printk. Instead, we explicitly mark it as initialized.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
Acked-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>

---

This patch was split from "kmsan: call KMSAN hooks where needed", as
requested by Andrey Konovalov. Petr Mladek has previously acked the
printk part of that patch, hence the Acked-by above.

v4:
 - split this patch away

Change-Id: Ibed60b0bdd25f8ae91acee5800b5328e78e0735a
Some functions are called from handwritten assembly, and therefore don't
have their arguments' metadata fully set up by the instrumentation code.
Mark them with __no_sanitize_memory to avoid false positives from
spreading further.
Certain functions perform task switching, so that the value of |current|
is different as they proceed. Because KMSAN state pointer is only read
once at the beginning of the function, touching it after |current| has
changed may be dangerous.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
---
v3:
 - removed TODOs from comments

v4:
 - updated the comments, dropped __no_sanitize_memory from idle_cpu(),
   sched_init(), profile_tick()
 - split away the uprobes part as requested by Andrey Konovalov

v6:
 - noinstr functions don't need KMSAN annotations
 - get_irq_regs() has moved to include/asm-generic/irq_regs.h

Change-Id: I684d23dac5a22eb0a4cea71993cb934302b17cea
This is a hack to reduce stackdepot pressure.

struct mmu_gather contains 7 1-bit fields packed into a 32-bit unsigned
int value. The remaining 25 bits remain uninitialized and are never used,
but KMSAN updates the origin for them in zap_pXX_range() in mm/memory.c,
thus creating very long origin chains. This is technically correct, but
consumes too much memory.

Unpoisoning the whole structure will prevent creating such chains.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>

---

v4:
 - removed a TODO, updated patch description

Change-Id: I22a201e7e4f67ed74f8129072f12e5351b26103a
Unless stated otherwise (by explicitly calling __memcpy(), __memset() or
__memmove()) we want all string functions to call their __msan_ versions
(e.g. __msan_memcpy() instead of memcpy()), so that shadow and origin
values are updated accordingly.

Bootloader must still use the default string functions to avoid crashes.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org
---
v3:
 - use default string functions in the bootloader
v4:
 - include kmsan-checks.h into compiler.h
 - also handle memset() and memmove()
 - fix #64
v5:
 - don't compile memset() and memmove() under KMSAN

Change-Id: Ib2512ce5aa8d457453dd38caa12f58f002166813
kmsan_initialize_shadow() creates metadata pages for mappings created
at boot time.

kmsan_initialize() initializes the bookkeeping for init_task and enables
KMSAN.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Marco Elver <elver@google.com>
Cc: linux-mm@kvack.org
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>

---

Change-Id: Ie3af251d629b911668f8651d868c544f3c11209f
Make KMSAN usable by adding the necessary Makefile bits.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---
This patch was previously called "kmsan: Changing existing files to
enable KMSAN builds". Logically unrelated parts of it were split away.

v4:
 - split away changes to init/main.c as requested by Andrey Konovalov

Change-Id: I37e0b7f2d2f2b0aeac5753ff9d6b411485fc374e
The random number generator may use uninitialized memory, but it may not
return uninitialized values. Unpoison the output buffer in
_extract_crng() to prevent false reports.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Eric Van Hensbergen <ericvh@gmail.com>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: linux-mm@kvack.org
Reviewed-by: Andrey Konovalov <andreyknvl@google.com>

---
This patch was previously known as "kmsan: unpoisoning buffers from
devices etc.", but it turned out to be possible to drop most of the
annotations from that patch, so it only relates to /dev/random now.

Change-Id: Id460e7a86ce564f1357469f53d0c7410ca08f0e9
If vring doesn't use the DMA API, KMSAN is unable to tell whether the
memory is initialized by hardware. Explicitly call kmsan_handle_dma()
from vring_map_one_sg() in this case to prevent false positives.

Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: linux-mm@kvack.org
---

v6:
 - fix kmsan_handle_dma() call to match the new prototype

Change-Id: Icc8678289b7084139320fc503898a67aa9803458
Disable the efficient 8-byte reading under KMSAN to avoid false positives.

Signed-off-by: Alexander Potapenko <glider@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-mm@kvack.org

---

v4:
 - actually disable the optimization under KMSAN via max=0
 - use IS_ENABLED as requested by Marco Elver

Change-Id: I25d1acf5c3df6eff85894cd94f5ddbe93308271c
ramosian-glider and others added 20 commits September 8, 2020 13:44
Call kmsan_copy_to_user() instead of kmsan_check_memory()

Signed-off-by: Alexander Potapenko <glider@google.com>
With KMSAN enabled the pages do not fit into 64 bytes anymore

Signed-off-by: Alexander Potapenko <glider@google.com>
It's not used anymore.

Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Drop the ancient Clang build instruction
LLVM implemented a recent "libcall optimization" that lowers calls to
`sprintf(dest, "%s", str)` where the return value is used to
`stpcpy(dest, str) - dest`. This generally avoids the machinery involved
in parsing format strings.  `stpcpy` is just like `strcpy` except it
returns the pointer to the new tail of `dest`.  This optimization was
introduced into clang-12.

Implement this so that we don't observe linkage failures due to missing
symbol definitions for `stpcpy`.

Similar to last year's fire drill with:
commit 5f074f3 ("lib/string.c: implement a basic bcmp")

The kernel is somewhere between a "freestanding" environment (no full libc)
and "hosted" environment (many symbols from libc exist with the same
type, function signature, and semantics).

As H. Peter Anvin notes, there's not really a great way to inform the
compiler that you're targeting a freestanding environment but would like
to opt-in to some libcall optimizations (see pr/47280 below), rather than
opt-out.

Arvind notes, -fno-builtin-* behaves slightly differently between GCC
and Clang, and Clang is missing many __builtin_* definitions, which I
consider a bug in Clang and am working on fixing.

Masahiro summarizes the subtle distinction between compilers justly:
  To prevent transformation from foo() into bar(), there are two ways in
  Clang to do that; -fno-builtin-foo, and -fno-builtin-bar.  There is
  only one in GCC; -fno-buitin-foo.

(Any difference in that behavior in Clang is likely a bug from a missing
__builtin_* definition.)

Masahiro also notes:
  We want to disable optimization from foo() to bar(),
  but we may still benefit from the optimization from
  foo() into something else. If GCC implements the same transform, we
  would run into a problem because it is not -fno-builtin-bar, but
  -fno-builtin-foo that disables that optimization.

  In this regard, -fno-builtin-foo would be more future-proof than
  -fno-built-bar, but -fno-builtin-foo is still potentially overkill. We
  may want to prevent calls from foo() being optimized into calls to
  bar(), but we still may want other optimization on calls to foo().

It seems that compilers today don't quite provide the fine grain control
over which libcall optimizations pseudo-freestanding environments would
prefer.

Finally, Kees notes that this interface is unsafe, so we should not
encourage its use.  As such, I've removed the declaration from any
header, but it still needs to be exported to avoid linkage errors in
modules.

Reported-by: Sami Tolvanen <samitolvanen@google.com>
Suggested-by: Andy Lavr <andy.lavr@gmail.com>
Suggested-by: Arvind Sankar <nivedita@alum.mit.edu>
Suggested-by: Joe Perches <joe@perches.com>
Suggested-by: Kees Cook <keescook@chromium.org>
Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Link: https://bugs.llvm.org/show_bug.cgi?id=47162
Link: https://bugs.llvm.org/show_bug.cgi?id=47280
Link: ClangBuiltLinux/linux#1126
Link: https://man7.org/linux/man-pages/man3/stpcpy.3.html
Link: https://pubs.opengroup.org/onlinepubs/9699919799/functions/stpcpy.html
Link: https://reviews.llvm.org/D85963
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Looks like it is impossible to go without one of the POLY1305
implementations, and even CONFIG_CRYPTO_POLY1305_X86_64 has a part
implemented in assembly. KMSAN can't comprehend that one, so we have to
unpoison the memory modified by the assembly code.

Signed-off-by: Alexander Potapenko <glider@google.com>
Can't really remember why we needed them in the first place.
Couldn't find them in the compiler pass either.

Signed-off-by: Alexander Potapenko <glider@google.com>
snd_usb_pipe_sanity_check() is a great function, so let's move it into
the USB core so that other parts of the kernel, including the USB core,
can call it.

Name it usb_pipe_type_check() to match the existing
usb_urb_ep_type_check() call, which now uses this function.

Cc: Jaroslav Kysela <perex@perex.cz>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Eli Billauer <eli.billauer@gmail.com>
Cc: Emiliano Ingrassia <ingrassia@epigenesys.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Alexander Tsoy <alexander@tsoy.me>
Cc: "Geoffrey D. Bennett" <g@b4.vu>
Cc: Jussi Laako <jussi@sonarnerd.net>
Cc: Nick Kossifidis <mickflemm@gmail.com>
Cc: Dmitry Panchenko <dmitry@d-systems.ee>
Cc: Chris Wulff <crwulff@gmail.com>
Cc: Jesus Ramos <jesus-ramos@live.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20200914153756.3412156-2-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
New core functions to make sending/receiving USB control messages easier
and saner.

In discussions, it turns out that the large majority of users of
usb_control_msg() do so in potentially incorrect ways.  The most common
issue is where a "short" message is received, yet never detected
properly due to "incorrect" error handling.

Handle all of this in the USB core with two new functions to try to make
working with USB control messages simpler.

No more need for dynamic data, messages can be on the stack, and only
"complete" send/receive will work without causing an error.

Link: https://lore.kernel.org/r/20200914153756.3412156-3-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
They need to specify how memory is to be allocated,
as control messages need to work in contexts that require GFP_NOIO.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Link: https://lore.kernel.org/r/20200923134348.23862-9-oneukum@suse.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
@google-cla
Copy link

google-cla bot commented Oct 2, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@thazhemadam
Copy link
Contributor Author

@googlebot I signed it!

@google-cla
Copy link

google-cla bot commented Oct 2, 2020

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@xairy
Copy link
Collaborator

xairy commented Oct 2, 2020

This doesn't look like anything KMSAN-specific. Should it rather be sent as a normal kernel patch?

@thazhemadam
Copy link
Contributor Author

It is a normal patch, but it hasn't made its way to the upstream kernel yet.
These patches contain new APIs that can fix a number of uninit bugs.

Since syzbot doesn't allow testing of KMSAN bugs on forks of the KMSAN kernel, I had asked @ramosian-glider if there was any way he could try to apply these patches so I could test the bug, and he asked me to open a PR.

@dvyukov
Copy link
Collaborator

dvyukov commented Oct 2, 2020

Since syzbot doesn't allow testing of KMSAN bugs on forks of the KMSAN kernel

syzbot allows testing with a custom patch on top of the tree. So you can submit all this in a patch testing request for any syzbot bug. It will be applied on top of KMSAN tree and you will get the same effect.

@thazhemadam
Copy link
Contributor Author

thazhemadam commented Oct 2, 2020

syzbot allows testing with a custom patch on top of the tree. So you can submit all this in a patch testing request for any syzbot bug. It will be applied on top of KMSAN tree and you will get the same effect.

I just thought since there seem to a few bugs that could be fixed with these patches, it might be generally helpful to have them (mainly the first 3 which are about the new API) applied to the KMSAN tree. No issues, I'll do that, instead.

@dvyukov
Copy link
Collaborator

dvyukov commented Oct 2, 2020

I just thought since there seem to a few bugs that could be fixed with these patches, it might be generally helpful to have them

If they fix some very frequent crashers, then I guess we could apply them as a short-term fix.
I will leave this to @ramosian-glider then, if he wants to maintain additional patches in the tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants