All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarmo Tiitto <jarmo.tiitto@gmail.com>
To: Sami Tolvanen <samitolvanen@google.com>,
	Bill Wendling <wcw@google.com>, Kees Cook <keescook@chromium.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	clang-built-linux@googlegroups.com, linux-kernel@vger.kernel.org
Cc: Jarmo Tiitto <jarmo.tiitto@gmail.com>, morbo@google.com
Subject: [PATCH v3 1/1] pgo: Fix allocate_node() v2
Date: Fri,  4 Jun 2021 19:58:20 +0300	[thread overview]
Message-ID: <20210604165819.7947-1-jarmo.tiitto@gmail.com> (raw)

When clang instrumentation eventually calls allocate_node()
the struct llvm_prf_data *p argument tells us from what section
we should reserve the vnode: It either points into vmlinux's
core __llvm_prf_data section or some loaded module's
__llvm_prf_data section.

But since we don't have access to corresponding
__llvm_prf_vnds section(s) for any module, the function
should return just NULL and ignore any profiling attempts
from modules for now.

Signed-off-by: Jarmo Tiitto <jarmo.tiitto@gmail.com>
---
Based on Kees and others feedback here is v3 patch
that clarifies why the current checks in allocate_node()
are flawed. I did fair amount of KGDB time on it.

The commit is based on kees/for-next/clang/features tree,
hopefully this is ok. Should I have based it on linux-next
instead?

I grep -R'd where the memory_contains() can be found and it is only
found in #include <asm-generic/sections.h>

I cross my fingers and await if this is my first accepted patch. :-)
---
 kernel/pgo/instrument.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/kernel/pgo/instrument.c b/kernel/pgo/instrument.c
index 0e07ee1b17d9..c69b4f7ebaad 100644
--- a/kernel/pgo/instrument.c
+++ b/kernel/pgo/instrument.c
@@ -18,6 +18,7 @@
 
 #define pr_fmt(fmt)	"pgo: " fmt
 
+#include <asm-generic/sections.h>
 #include <linux/bitops.h>
 #include <linux/kernel.h>
 #include <linux/export.h>
@@ -55,17 +56,21 @@ void prf_unlock(unsigned long flags)
 static struct llvm_prf_value_node *allocate_node(struct llvm_prf_data *p,
 						 u32 index, u64 value)
 {
-	if (&__llvm_prf_vnds_start[current_node + 1] >= __llvm_prf_vnds_end)
-		return NULL; /* Out of nodes */
-
-	current_node++;
-
-	/* Make sure the node is entirely within the section */
-	if (&__llvm_prf_vnds_start[current_node] >= __llvm_prf_vnds_end ||
-	    &__llvm_prf_vnds_start[current_node + 1] > __llvm_prf_vnds_end)
+	const int max_vnds = prf_vnds_count();
+
+	/*
+	 * Check that p is within vmlinux __llvm_prf_data section.
+	 * If not, don't allocate since we can't handle modules yet.
+	 */
+	if (!memory_contains(__llvm_prf_data_start,
+		__llvm_prf_data_end, p, sizeof(*p)))
 		return NULL;
 
-	return &__llvm_prf_vnds_start[current_node];
+	if (WARN_ON_ONCE(current_node >= max_vnds))
+		return NULL; /* Out of nodes */
+
+	/* reserve vnode for vmlinux */
+	return &__llvm_prf_vnds_start[current_node++];
 }
 
 /*

base-commit: 5d0cda65918279ada060417c5fecb7e86ccb3def
-- 
2.31.1


             reply	other threads:[~2021-06-04 17:04 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 16:58 Jarmo Tiitto [this message]
2021-06-04 17:04 ` [PATCH v3 1/1] pgo: Fix allocate_node() v2 Nathan Chancellor
2021-06-04 17:28 ` Nick Desaulniers
2021-06-04 17:58 ` Kees Cook
2021-06-04 18:06 ` Kees Cook
2021-06-05 17:15   ` Jarmo Tiitto
2021-06-05 23:45     ` Fangrui Song

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=20210604165819.7947-1-jarmo.tiitto@gmail.com \
    --to=jarmo.tiitto@gmail.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=samitolvanen@google.com \
    --cc=wcw@google.com \
    /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: link
Be 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.