From: Andrew Morton <akpm@linux-foundation.org> To: akpm@linux-foundation.org, andreyknvl@google.com, dvyukov@google.com, elver@google.com, glider@google.com, linux-mm@kvack.org, mm-commits@vger.kernel.org, penguin-kernel@i-love.sakura.ne.jp, torvalds@linux-foundation.org Subject: [patch 03/25] kcov: check kcov_softirq in kcov_remote_stop() Date: Wed, 10 Jun 2020 18:41:28 -0700 [thread overview] Message-ID: <20200611014128.5iEXzjAG-%akpm@linux-foundation.org> (raw) In-Reply-To: <20200610184053.3fa7368ab80e23bfd44de71f@linux-foundation.org> From: Andrey Konovalov <andreyknvl@google.com> Subject: kcov: check kcov_softirq in kcov_remote_stop() kcov_remote_stop() should check that the corresponding kcov_remote_start() actually found the specified remote handle and started collecting coverage. This is done by checking the per thread kcov_softirq flag. A particular failure scenario where this was observed involved a softirq with a remote coverage collection section coming between check_kcov_mode() and the access to t->kcov_area in __sanitizer_cov_trace_pc(). In that softirq kcov_remote_start() bailed out after kcov_remote_find() check, but the matching kcov_remote_stop() didn't check if kcov_remote_start() succeeded, and overwrote per thread kcov parameters with invalid (zero) values. Link: http://lkml.kernel.org/r/fcd1cd16eac1d2c01a66befd8ea4afc6f8d09833.1591576806.git.andreyknvl@google.com Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts") Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Reviewed-by: Dmitry Vyukov <dvyukov@google.com> Cc: Alexander Potapenko <glider@google.com> Cc: Marco Elver <elver@google.com> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- kernel/kcov.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) --- a/kernel/kcov.c~kcov-check-kcov_softirq-in-kcov_remote_stop +++ a/kernel/kcov.c @@ -427,7 +427,8 @@ void kcov_task_exit(struct task_struct * * WARN_ON(!kcov->remote && kcov->t != t); * * For KCOV_REMOTE_ENABLE devices, the exiting task is either: - * 2. A remote task between kcov_remote_start() and kcov_remote_stop(). + * + * 1. A remote task between kcov_remote_start() and kcov_remote_stop(). * In this case we should print a warning right away, since a task * shouldn't be exiting when it's in a kcov coverage collection * section. Here t points to the task that is collecting remote @@ -437,7 +438,7 @@ void kcov_task_exit(struct task_struct * * WARN_ON(kcov->remote && kcov->t != t); * * 2. The task that created kcov exiting without calling KCOV_DISABLE, - * and then again we can make sure that t->kcov->t == t: + * and then again we make sure that t->kcov->t == t: * WARN_ON(kcov->remote && kcov->t != t); * * By combining all three checks into one we get: @@ -764,7 +765,7 @@ static const struct file_operations kcov * Internally, kcov_remote_start() looks up the kcov device associated with the * provided handle, allocates an area for coverage collection, and saves the * pointers to kcov and area into the current task_struct to allow coverage to - * be collected via __sanitizer_cov_trace_pc() + * be collected via __sanitizer_cov_trace_pc(). * In turns kcov_remote_stop() clears those pointers from task_struct to stop * collecting coverage and copies all collected coverage into the kcov area. */ @@ -972,16 +973,25 @@ void kcov_remote_stop(void) local_irq_restore(flags); return; } - kcov = t->kcov; - area = t->kcov_area; - size = t->kcov_size; - sequence = t->kcov_sequence;
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org> To: akpm@linux-foundation.org, andreyknvl@google.com, dvyukov@google.com, elver@google.com, glider@google.com, linux-mm@kvack.org, mm-commits@vger.kernel.org, penguin-kernel@i-love.sakura.ne.jp, torvalds@linux-foundation.org Subject: [patch 03/25] kcov: check kcov_softirq in kcov_remote_stop() Date: Wed, 10 Jun 2020 18:41:28 -0700 [thread overview] Message-ID: <20200611014128.5iEXzjAG-%akpm@linux-foundation.org> (raw) In-Reply-To: <20200610184053.3fa7368ab80e23bfd44de71f@linux-foundation.org> From: Andrey Konovalov <andreyknvl@google.com> Subject: kcov: check kcov_softirq in kcov_remote_stop() kcov_remote_stop() should check that the corresponding kcov_remote_start() actually found the specified remote handle and started collecting coverage. This is done by checking the per thread kcov_softirq flag. A particular failure scenario where this was observed involved a softirq with a remote coverage collection section coming between check_kcov_mode() and the access to t->kcov_area in __sanitizer_cov_trace_pc(). In that softirq kcov_remote_start() bailed out after kcov_remote_find() check, but the matching kcov_remote_stop() didn't check if kcov_remote_start() succeeded, and overwrote per thread kcov parameters with invalid (zero) values. Link: http://lkml.kernel.org/r/fcd1cd16eac1d2c01a66befd8ea4afc6f8d09833.1591576806.git.andreyknvl@google.com Fixes: 5ff3b30ab57d ("kcov: collect coverage from interrupts") Signed-off-by: Andrey Konovalov <andreyknvl@google.com> Reviewed-by: Dmitry Vyukov <dvyukov@google.com> Cc: Alexander Potapenko <glider@google.com> Cc: Marco Elver <elver@google.com> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- kernel/kcov.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) --- a/kernel/kcov.c~kcov-check-kcov_softirq-in-kcov_remote_stop +++ a/kernel/kcov.c @@ -427,7 +427,8 @@ void kcov_task_exit(struct task_struct * * WARN_ON(!kcov->remote && kcov->t != t); * * For KCOV_REMOTE_ENABLE devices, the exiting task is either: - * 2. A remote task between kcov_remote_start() and kcov_remote_stop(). + * + * 1. A remote task between kcov_remote_start() and kcov_remote_stop(). * In this case we should print a warning right away, since a task * shouldn't be exiting when it's in a kcov coverage collection * section. Here t points to the task that is collecting remote @@ -437,7 +438,7 @@ void kcov_task_exit(struct task_struct * * WARN_ON(kcov->remote && kcov->t != t); * * 2. The task that created kcov exiting without calling KCOV_DISABLE, - * and then again we can make sure that t->kcov->t == t: + * and then again we make sure that t->kcov->t == t: * WARN_ON(kcov->remote && kcov->t != t); * * By combining all three checks into one we get: @@ -764,7 +765,7 @@ static const struct file_operations kcov * Internally, kcov_remote_start() looks up the kcov device associated with the * provided handle, allocates an area for coverage collection, and saves the * pointers to kcov and area into the current task_struct to allow coverage to - * be collected via __sanitizer_cov_trace_pc() + * be collected via __sanitizer_cov_trace_pc(). * In turns kcov_remote_stop() clears those pointers from task_struct to stop * collecting coverage and copies all collected coverage into the kcov area. */ @@ -972,16 +973,25 @@ void kcov_remote_stop(void) local_irq_restore(flags); return; } - kcov = t->kcov; - area = t->kcov_area; - size = t->kcov_size; - sequence = t->kcov_sequence; - + /* + * When in softirq, check if the corresponding kcov_remote_start() + * actually found the remote handle and started collecting coverage. + */ + if (in_serving_softirq() && !t->kcov_softirq) { + local_irq_restore(flags); + return; + } + /* Make sure that kcov_softirq is only set when in softirq. */ if (WARN_ON(!in_serving_softirq() && t->kcov_softirq)) { local_irq_restore(flags); return; } + kcov = t->kcov; + area = t->kcov_area; + size = t->kcov_size; + sequence = t->kcov_sequence; + kcov_stop(t); if (in_serving_softirq()) { t->kcov_softirq = 0; _
next prev parent reply other threads:[~2020-06-11 1:41 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-11 1:40 incoming Andrew Morton 2020-06-11 1:41 ` [patch 01/25] khugepaged: selftests: fix timeout condition in wait_for_scan() Andrew Morton 2020-06-11 1:41 ` [patch 02/25] scripts/spelling: add a few more typos Andrew Morton 2020-06-11 1:41 ` Andrew Morton [this message] 2020-06-11 1:41 ` [patch 03/25] kcov: check kcov_softirq in kcov_remote_stop() Andrew Morton 2020-06-11 1:41 ` [patch 04/25] lib/lz4/lz4_decompress.c: document deliberate use of `&' Andrew Morton 2020-06-11 1:41 ` [patch 05/25] nilfs2: fix null pointer dereference at nilfs_segctor_do_construct() Andrew Morton 2020-06-11 1:41 ` Andrew Morton 2020-06-11 1:41 ` [patch 06/25] checkpatch: correct check for kernel parameters doc Andrew Morton 2020-06-11 1:41 ` [patch 07/25] lib: fix bitmap_parse() on 64-bit big endian archs Andrew Morton 2020-06-11 1:41 ` [patch 08/25] mm/debug_vm_pgtable: fix kernel crash by checking for THP support Andrew Morton 2020-06-11 1:41 ` [patch 09/25] ocfs2: fix spelling mistake and grammar Andrew Morton 2020-06-11 1:41 ` [patch 10/25] mm: add comments on pglist_data zones Andrew Morton 2020-06-11 1:41 ` [patch 11/25] lib: test get_count_order/long in test_bitops.c Andrew Morton 2020-06-11 1:41 ` [patch 12/25] stacktrace: cleanup inconsistent variable type Andrew Morton 2020-06-11 1:41 ` [patch 13/25] kernel: move use_mm/unuse_mm to kthread.c Andrew Morton 2020-06-11 1:41 ` Andrew Morton 2020-06-11 1:42 ` [patch 14/25] " Andrew Morton 2020-06-11 1:42 ` [patch 15/25] kernel: better document the use_mm/unuse_mm API contract Andrew Morton 2020-06-11 1:42 ` [patch 16/25] kernel: set USER_DS in kthread_use_mm Andrew Morton 2020-06-11 1:42 ` [patch 17/25] mm/madvise: pass task and mm to do_madvise Andrew Morton 2020-06-11 1:42 ` [patch 18/25] mm/madvise: introduce process_madvise() syscall: an external memory hinting API Andrew Morton 2020-06-11 1:42 ` [patch 19/25] mm/madvise: check fatal signal pending of target process Andrew Morton 2020-06-11 1:42 ` [patch 20/25] pid: move pidfd_get_pid() to pid.c Andrew Morton 2020-06-11 1:42 ` [patch 21/25] mm/madvise: support both pid and pidfd for process_madvise Andrew Morton 2020-06-11 1:42 ` [patch 22/25] mm/madvise: allow KSM hints for remote API Andrew Morton 2020-06-11 1:42 ` [patch 23/25] mm: support vector address ranges for process_madvise Andrew Morton 2020-06-11 1:42 ` [patch 24/25] mm: use only pidfd for process_madvise syscall Andrew Morton 2020-06-11 2:09 ` Linus Torvalds 2020-06-11 3:10 ` Minchan Kim 2020-06-11 1:42 ` [patch 25/25] mm/madvise.c: remove duplicated include Andrew Morton 2020-06-11 5:25 ` [to-be-updated] mm-pass-task-and-mm-to-do_madvise.patch removed from -mm tree Andrew Morton 2020-06-11 5:26 ` [to-be-updated] mm-introduce-external-memory-hinting-api.patch " Andrew Morton 2020-06-11 5:26 ` [to-be-updated] mm-check-fatal-signal-pending-of-target-process.patch " Andrew Morton 2020-06-11 5:26 ` [to-be-updated] pid-move-pidfd_get_pid-function-to-pidc.patch " Andrew Morton 2020-06-11 5:26 ` [to-be-updated] mm-support-both-pid-and-pidfd-for-process_madvise.patch " Andrew Morton 2020-06-11 5:26 ` [to-be-updated] mm-madvise-allow-ksm-hints-for-remote-api.patch " Andrew Morton 2020-06-11 5:26 ` [to-be-updated] mm-support-vector-address-ranges-for-process_madvise.patch " Andrew Morton 2020-06-11 5:26 ` [to-be-updated] mm-use-only-pidfd-for-process_madvise-syscall.patch " Andrew Morton
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200611014128.5iEXzjAG-%akpm@linux-foundation.org \ --to=akpm@linux-foundation.org \ --cc=andreyknvl@google.com \ --cc=dvyukov@google.com \ --cc=elver@google.com \ --cc=glider@google.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mm-commits@vger.kernel.org \ --cc=penguin-kernel@i-love.sakura.ne.jp \ --cc=torvalds@linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.