* [PATCH v3 1/3] kallsyms: move kallsyms_show_value() out of kallsyms.c [not found] <CGME20230605040744epcas5p21968bee09fba5c3505a729fe2f57c507@epcas5p2.samsung.com> @ 2023-06-05 4:07 ` Maninder Singh [not found] ` <CGME20230605040753epcas5p2640ca20e5fe0f628926d5cba16d8270a@epcas5p2.samsung.com> ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Maninder Singh @ 2023-06-05 4:07 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 function kallsyms_show_value() is used by other parts like modules_open(), kprobes_read() etc. which can work in case of !KALLSYMS also. e.g. as of now lsmod do not show module address if KALLSYMS is disabled. since kallsyms_show_value() defination is not present, it returns false in !KALLSYMS. / # lsmod test 12288 0 - Live 0x0000000000000000 (O) So kallsyms_show_value() can be made generic without dependency on KALLSYMS. Thus moving out function to a new file knosyms.c. With this patch code is just moved to new file and no functional change. Next patch will enable defination of function for all cases. 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 v1 -> v2: separate out bpf and kallsyms change v2 -> v3: make kallsym changes in2 patches, non functional and functional change kernel/Makefile | 2 +- kernel/kallsyms.c | 35 ---------------------------------- kernel/knosyms.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 36 deletions(-) create mode 100644 kernel/knosyms.c diff --git a/kernel/Makefile b/kernel/Makefile index f9e3fd9195d9..918d3e9b14bc 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 8193e947aa10..0f82c3d5a57d 100644 --- a/kernel/kallsyms.c +++ b/kernel/kallsyms.c @@ -907,41 +907,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..9e2c72a89ea5 --- /dev/null +++ b/kernel/knosyms.c @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2023 Samsung Electronics Co., Ltd + * + * A split of kernel/kallsyms.c + * It will contain few generic function definations independent of config KALLSYMS. + */ + +#include <linux/kallsyms.h> +#include <linux/security.h> + +#ifdef CONFIG_KALLSYMS +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; + } +} +#endif -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <CGME20230605040753epcas5p2640ca20e5fe0f628926d5cba16d8270a@epcas5p2.samsung.com>]
* [PATCH v3 2/3] kallsyms: make kallsyms_show_value() as generic function [not found] ` <CGME20230605040753epcas5p2640ca20e5fe0f628926d5cba16d8270a@epcas5p2.samsung.com> @ 2023-06-05 4:07 ` Maninder Singh 0 siblings, 0 replies; 5+ messages in thread From: Maninder Singh @ 2023-06-05 4:07 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 This change makes function kallsyms_show_value() as generic function without dependency on CONFIG_KALLSYMS. Now module address will be displayed with lsmod and /proc/modules. 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) 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> --- include/linux/kallsyms.h | 11 +++-------- kernel/knosyms.c | 2 -- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h index 1037f4957caa..c3f075e8f60c 100644 --- a/include/linux/kallsyms.h +++ b/include/linux/kallsyms.h @@ -65,6 +65,9 @@ static inline void *dereference_symbol_descriptor(void *ptr) return ptr; } +/* How and when do we show kallsyms values? */ +extern bool kallsyms_show_value(const struct cred *cred); + #ifdef CONFIG_KALLSYMS unsigned long kallsyms_sym_address(int idx); int kallsyms_on_each_symbol(int (*fn)(void *, const char *, unsigned long), @@ -94,9 +97,6 @@ extern int sprint_backtrace_build_id(char *buffer, unsigned long address); int lookup_symbol_name(unsigned long addr, char *symname); -/* How and when do we show kallsyms values? */ -extern bool kallsyms_show_value(const struct cred *cred); - #else /* !CONFIG_KALLSYMS */ static inline unsigned long kallsyms_lookup_name(const char *name) @@ -154,11 +154,6 @@ static inline int lookup_symbol_name(unsigned long addr, char *symname) 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/knosyms.c b/kernel/knosyms.c index 9e2c72a89ea5..830905b0986a 100644 --- a/kernel/knosyms.c +++ b/kernel/knosyms.c @@ -9,7 +9,6 @@ #include <linux/kallsyms.h> #include <linux/security.h> -#ifdef CONFIG_KALLSYMS static inline int kallsyms_for_perf(void) { #ifdef CONFIG_PERF_EVENTS @@ -45,4 +44,3 @@ bool kallsyms_show_value(const struct cred *cred) return false; } } -#endif -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <CGME20230605040801epcas5p2ca850464882841a0a5748e217542a10a@epcas5p2.samsung.com>]
* [PATCH v3 3/3] bpf: make bpf_dump_raw_ok() based on CONFIG_KALLSYMS [not found] ` <CGME20230605040801epcas5p2ca850464882841a0a5748e217542a10a@epcas5p2.samsung.com> @ 2023-06-05 4:07 ` Maninder Singh 2023-06-05 11:46 ` Leizhen (ThunderTown) 0 siblings, 1 reply; 5+ messages in thread From: Maninder Singh @ 2023-06-05 4:07 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 bpf_dump_raw_ok() depends on kallsyms_show_value() and we already have a false definition for the !CONFIG_KALLSYMS case. But we have expanded kallsyms_show_value() to work for !CONFIG_KALLSYMS case also in previous patch. And so to make the code easier to follow just provide a direct !CONFIG_KALLSYMS definition for bpf_dump_raw_ok() as well. As it is heavily dependent on KALLSYMS and checking based on kallsyms_show_value() will not work now. 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> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> --- 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 [flat|nested] 5+ messages in thread
* Re: [PATCH v3 3/3] bpf: make bpf_dump_raw_ok() based on CONFIG_KALLSYMS 2023-06-05 4:07 ` [PATCH v3 3/3] bpf: make bpf_dump_raw_ok() based on CONFIG_KALLSYMS Maninder Singh @ 2023-06-05 11:46 ` Leizhen (ThunderTown) 0 siblings, 0 replies; 5+ messages in thread From: Leizhen (ThunderTown) @ 2023-06-05 11:46 UTC (permalink / raw) To: Maninder Singh, ast, daniel, john.fastabend, andrii, martin.lau, song, yhs, kpsingh, sdf, haoluo, jolsa, mcgrof, boqun.feng, vincenzopalazzodev, ojeda, jgross, brauner, michael.christie, samitolvanen, glider, peterz, keescook, stephen.s.brennan, alan.maguire, pmladek Cc: linux-kernel, bpf, Onkarnath On 2023/6/5 12:07, Maninder Singh wrote: > bpf_dump_raw_ok() depends on kallsyms_show_value() and we already > have a false definition for the !CONFIG_KALLSYMS case. But we have > expanded kallsyms_show_value() to work for !CONFIG_KALLSYMS case also > in previous patch. > > And so to make the code easier to follow just provide a direct > !CONFIG_KALLSYMS definition for bpf_dump_raw_ok() as well. > > As it is heavily dependent on KALLSYMS and checking based on > kallsyms_show_value() will not work now. This patch needs to be swapped with 2/3. To avoid unnecessary trouble for people using "git bisect", assume that they just fall back to patch 2/3. > > 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> > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > --- > 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); > -- Regards, Zhen Lei ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/3] kallsyms: move kallsyms_show_value() out of kallsyms.c 2023-06-05 4:07 ` [PATCH v3 1/3] kallsyms: move kallsyms_show_value() out of kallsyms.c Maninder Singh [not found] ` <CGME20230605040753epcas5p2640ca20e5fe0f628926d5cba16d8270a@epcas5p2.samsung.com> [not found] ` <CGME20230605040801epcas5p2ca850464882841a0a5748e217542a10a@epcas5p2.samsung.com> @ 2023-06-05 11:32 ` Leizhen (ThunderTown) 2 siblings, 0 replies; 5+ messages in thread From: Leizhen (ThunderTown) @ 2023-06-05 11:32 UTC (permalink / raw) To: Maninder Singh, ast, daniel, john.fastabend, andrii, martin.lau, song, yhs, kpsingh, sdf, haoluo, jolsa, mcgrof, boqun.feng, vincenzopalazzodev, ojeda, jgross, brauner, michael.christie, samitolvanen, glider, peterz, keescook, stephen.s.brennan, alan.maguire, pmladek Cc: linux-kernel, bpf, Onkarnath On 2023/6/5 12:07, Maninder Singh wrote: > function kallsyms_show_value() is used by other parts > like modules_open(), kprobes_read() etc. which can work in case of > !KALLSYMS also. > > e.g. as of now lsmod do not show module address if KALLSYMS is disabled. > since kallsyms_show_value() defination is not present, it returns false > in !KALLSYMS. > > / # lsmod > test 12288 0 - Live 0x0000000000000000 (O) > > So kallsyms_show_value() can be made generic > without dependency on KALLSYMS. > > Thus moving out function to a new file knosyms.c. > > With this patch code is just moved to new file > and no functional change. > > Next patch will enable defination of function for all cases. > > 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 > v1 -> v2: separate out bpf and kallsyms change > v2 -> v3: make kallsym changes in2 patches, non functional and > functional change > > kernel/Makefile | 2 +- > kernel/kallsyms.c | 35 ---------------------------------- > kernel/knosyms.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 49 insertions(+), 36 deletions(-) > create mode 100644 kernel/knosyms.c > > diff --git a/kernel/Makefile b/kernel/Makefile > index f9e3fd9195d9..918d3e9b14bc 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 8193e947aa10..0f82c3d5a57d 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -907,41 +907,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 Maybe it's better to have the words like 'common' in the file name. This file is also used when CONFIG_KALLSYMS=y, and knosyms is contradictory. > new file mode 100644 > index 000000000000..9e2c72a89ea5 > --- /dev/null > +++ b/kernel/knosyms.c > @@ -0,0 +1,48 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2023 Samsung Electronics Co., Ltd This copyright notice is a little inappropriate. Now it's just moved, not original. > + * > + * A split of kernel/kallsyms.c > + * It will contain few generic function definations independent of config KALLSYMS. > + */ > + > +#include <linux/kallsyms.h> > +#include <linux/security.h> > + > +#ifdef CONFIG_KALLSYMS > +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; > + } > +} > +#endif > -- Regards, Zhen Lei ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-05 11:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20230605040744epcas5p21968bee09fba5c3505a729fe2f57c507@epcas5p2.samsung.com>
2023-06-05 4:07 ` [PATCH v3 1/3] kallsyms: move kallsyms_show_value() out of kallsyms.c Maninder Singh
[not found] ` <CGME20230605040753epcas5p2640ca20e5fe0f628926d5cba16d8270a@epcas5p2.samsung.com>
2023-06-05 4:07 ` [PATCH v3 2/3] kallsyms: make kallsyms_show_value() as generic function Maninder Singh
[not found] ` <CGME20230605040801epcas5p2ca850464882841a0a5748e217542a10a@epcas5p2.samsung.com>
2023-06-05 4:07 ` [PATCH v3 3/3] bpf: make bpf_dump_raw_ok() based on CONFIG_KALLSYMS Maninder Singh
2023-06-05 11:46 ` Leizhen (ThunderTown)
2023-06-05 11:32 ` [PATCH v3 1/3] kallsyms: move kallsyms_show_value() out of kallsyms.c Leizhen (ThunderTown)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®