bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] bpf: make defination of bpf_dump_raw_ok based on CONFIG_KALLSYMS
       [not found] <CGME20230531084800epcas5p1c02d5a7ffcef2bb7cf626bf52beb0eee@epcas5p1.samsung.com>
@ 2023-05-31  8:47 ` Maninder Singh
       [not found]   ` <CGME20230531084808epcas5p4a782bf6bb5655f2d320c246057e7bb63@epcas5p4.samsung.com>
  0 siblings, 1 reply; 2+ messages in thread
From: Maninder Singh @ 2023-05-31  8:47 UTC (permalink / raw)
  To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
	kpsingh, sdf, haoluo, jolsa, thunder.leizhen, mcgrof, boqun.feng,
	vincenzopalazzodev, ojeda, jgross, brauner, michael.christie,
	samitolvanen, glider, peterz, keescook, stephen.s.brennan,
	alan.maguire, pmladek
  Cc: linux-kernel, bpf, Maninder Singh, Onkarnath

No functional change with this commit.

As of now bpf_dump_raw_ok() is dependent on kallsyms_show_value().
Rearranging the code to return false directly in defination of
bpf_dump_raw_ok() based on CONFIG_KALLSYMS.

@reason: next patch will make kallsyms_show_value() defination
generic and bpf_dump_raw_ok() will return true otherwise.

Thus make decision based on CONFIG rather than kallsyms_show_value(),
as bpf functionality is heavily dependent on KALLSYMS.

Co-developed-by: Onkarnath <onkarnath.1@samsung.com>
Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
---
v1 -> v2 : made separate patches for kallsyms and bpf

 include/linux/filter.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index bbce89937fde..1f237a3bb11a 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -923,13 +923,21 @@ bool bpf_jit_supports_kfunc_call(void);
 bool bpf_jit_supports_far_kfunc_call(void);
 bool bpf_helper_changes_pkt_data(void *func);
 
+/*
+ * Reconstruction of call-sites is dependent on kallsyms,
+ * thus make dump the same restriction.
+ */
+#ifdef CONFIG_KALLSYMS
 static inline bool bpf_dump_raw_ok(const struct cred *cred)
 {
-	/* Reconstruction of call-sites is dependent on kallsyms,
-	 * thus make dump the same restriction.
-	 */
 	return kallsyms_show_value(cred);
 }
+#else
+static inline bool bpf_dump_raw_ok(const struct cred *cred)
+{
+	return false;
+}
+#endif
 
 struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,
 				       const struct bpf_insn *patch, u32 len);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH v2 2/2] kallsyms: add kallsyms_show_value defination in all cases
       [not found]   ` <CGME20230531084808epcas5p4a782bf6bb5655f2d320c246057e7bb63@epcas5p4.samsung.com>
@ 2023-05-31  8:47     ` Maninder Singh
  0 siblings, 0 replies; 2+ messages in thread
From: Maninder Singh @ 2023-05-31  8:47 UTC (permalink / raw)
  To: ast, daniel, john.fastabend, andrii, martin.lau, song, yhs,
	kpsingh, sdf, haoluo, jolsa, thunder.leizhen, mcgrof, boqun.feng,
	vincenzopalazzodev, ojeda, jgross, brauner, michael.christie,
	samitolvanen, glider, peterz, keescook, stephen.s.brennan,
	alan.maguire, pmladek
  Cc: linux-kernel, bpf, Maninder Singh, Onkarnath

kallsyms_show_value return false if KALLSYMS is disabled,
but it is getting used in module.c also for fetching module address.

Thus when KALLSYMS is disabled, system will not print module
load address:

Earlier:
=======
/ # insmod  test.ko
/ # lsmod
test 12288 0 - Live 0x0000000000000000 (O)  // No Module Load address
/ #

With change:
==========
/ # insmod test.ko
/ # lsmod
test 12288 0 - Live 0xffff800000fc0000 (O)  // Module address
/ # cat /proc/modules
test 12288 0 - Live 0xffff800000fc0000 (O)

New file knosyms.c is added (name suggested by Kees Cook),
which includes common defination irrespective of CONFIG_KALLSYMS.

Co-developed-by: Onkarnath <onkarnath.1@samsung.com>
Signed-off-by: Onkarnath <onkarnath.1@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
---
earlier conversations:(then it has dependancy on other change, but that
was stashed from linux-next, now it can be pushed)
https://lkml.org/lkml/2022/5/11/212
https://lkml.org/lkml/2022/4/13/47

 include/linux/kallsyms.h | 10 +++-----
 kernel/Makefile          |  2 +-
 kernel/kallsyms.c        | 35 ---------------------------
 kernel/knosyms.c         | 52 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 43 deletions(-)
 create mode 100644 kernel/knosyms.c

diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index fe3c9993b5bf..3f3559ba0c56 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -24,6 +24,9 @@
 struct cred;
 struct module;
 
+/* How and when do we show kallsyms values? */
+extern bool kallsyms_show_value(const struct cred *cred);
+
 static inline int is_kernel_text(unsigned long addr)
 {
 	if (__is_kernel_text(addr))
@@ -95,8 +98,6 @@ extern int sprint_backtrace_build_id(char *buffer, unsigned long address);
 int lookup_symbol_name(unsigned long addr, char *symname);
 int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
 
-/* How and when do we show kallsyms values? */
-extern bool kallsyms_show_value(const struct cred *cred);
 
 #else /* !CONFIG_KALLSYMS */
 
@@ -160,11 +161,6 @@ static inline int lookup_symbol_attrs(unsigned long addr, unsigned long *size, u
 	return -ERANGE;
 }
 
-static inline bool kallsyms_show_value(const struct cred *cred)
-{
-	return false;
-}
-
 static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *, unsigned long),
 					  void *data)
 {
diff --git a/kernel/Makefile b/kernel/Makefile
index b69c95315480..76efc634b1a4 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -10,7 +10,7 @@ obj-y     = fork.o exec_domain.o panic.o \
 	    extable.o params.o \
 	    kthread.o sys_ni.o nsproxy.o \
 	    notifier.o ksysfs.o cred.o reboot.o \
-	    async.o range.o smpboot.o ucount.o regset.o
+	    async.o range.o smpboot.o ucount.o regset.o knosyms.o
 
 obj-$(CONFIG_USERMODE_DRIVER) += usermode_driver.o
 obj-$(CONFIG_MULTIUSER) += groups.o
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 77747391f49b..1dcb7051f852 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -961,41 +961,6 @@ late_initcall(bpf_ksym_iter_register);
 
 #endif /* CONFIG_BPF_SYSCALL */
 
-static inline int kallsyms_for_perf(void)
-{
-#ifdef CONFIG_PERF_EVENTS
-	extern int sysctl_perf_event_paranoid;
-	if (sysctl_perf_event_paranoid <= 1)
-		return 1;
-#endif
-	return 0;
-}
-
-/*
- * We show kallsyms information even to normal users if we've enabled
- * kernel profiling and are explicitly not paranoid (so kptr_restrict
- * is clear, and sysctl_perf_event_paranoid isn't set).
- *
- * Otherwise, require CAP_SYSLOG (assuming kptr_restrict isn't set to
- * block even that).
- */
-bool kallsyms_show_value(const struct cred *cred)
-{
-	switch (kptr_restrict) {
-	case 0:
-		if (kallsyms_for_perf())
-			return true;
-		fallthrough;
-	case 1:
-		if (security_capable(cred, &init_user_ns, CAP_SYSLOG,
-				     CAP_OPT_NOAUDIT) == 0)
-			return true;
-		fallthrough;
-	default:
-		return false;
-	}
-}
-
 static int kallsyms_open(struct inode *inode, struct file *file)
 {
 	/*
diff --git a/kernel/knosyms.c b/kernel/knosyms.c
new file mode 100644
index 000000000000..f2c8755e22e7
--- /dev/null
+++ b/kernel/knosyms.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Samsung Electronics Co., Ltd
+ *
+ * Author:
+ *	Maninder singh <maninder1.s@samsung.com>
+ *	Onkarnath <onkarnath.1@samsung.com>
+ *
+ * A split of kernel/kallsyms.c
+ * to contain few generic function definations independent of config KALLSYMS
+ * or defined under KALLSYMS disabled.
+ */
+
+#include <linux/kallsyms.h>
+#include <linux/security.h>
+
+static inline int kallsyms_for_perf(void)
+{
+#ifdef CONFIG_PERF_EVENTS
+	extern int sysctl_perf_event_paranoid;
+
+	if (sysctl_perf_event_paranoid <= 1)
+		return 1;
+#endif
+	return 0;
+}
+
+/*
+ * We show kallsyms information even to normal users if we've enabled
+ * kernel profiling and are explicitly not paranoid (so kptr_restrict
+ * is clear, and sysctl_perf_event_paranoid isn't set).
+ *
+ * Otherwise, require CAP_SYSLOG (assuming kptr_restrict isn't set to
+ * block even that).
+ */
+bool kallsyms_show_value(const struct cred *cred)
+{
+	switch (kptr_restrict) {
+	case 0:
+		if (kallsyms_for_perf())
+			return true;
+		fallthrough;
+	case 1:
+		if (security_capable(cred, &init_user_ns, CAP_SYSLOG,
+				     CAP_OPT_NOAUDIT) == 0)
+			return true;
+		fallthrough;
+	default:
+		return false;
+	}
+}
+
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-05-31  8:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20230531084800epcas5p1c02d5a7ffcef2bb7cf626bf52beb0eee@epcas5p1.samsung.com>
2023-05-31  8:47 ` [PATCH v2 1/2] bpf: make defination of bpf_dump_raw_ok based on CONFIG_KALLSYMS Maninder Singh
     [not found]   ` <CGME20230531084808epcas5p4a782bf6bb5655f2d320c246057e7bb63@epcas5p4.samsung.com>
2023-05-31  8:47     ` [PATCH v2 2/2] kallsyms: add kallsyms_show_value defination in all cases Maninder Singh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).