[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fb89d084-9ccb-ee90-25aa-43122e86d663@huawei.com>
Date: Mon, 5 Jun 2023 19:32:15 +0800
From: "Leizhen (ThunderTown)" <thunder.leizhen@...wei.com>
To: Maninder Singh <maninder1.s@...sung.com>, <ast@...nel.org>,
<daniel@...earbox.net>, <john.fastabend@...il.com>,
<andrii@...nel.org>, <martin.lau@...ux.dev>, <song@...nel.org>,
<yhs@...com>, <kpsingh@...nel.org>, <sdf@...gle.com>,
<haoluo@...gle.com>, <jolsa@...nel.org>, <mcgrof@...nel.org>,
<boqun.feng@...il.com>, <vincenzopalazzodev@...il.com>,
<ojeda@...nel.org>, <jgross@...e.com>, <brauner@...nel.org>,
<michael.christie@...cle.com>, <samitolvanen@...gle.com>,
<glider@...gle.com>, <peterz@...radead.org>,
<keescook@...omium.org>, <stephen.s.brennan@...cle.com>,
<alan.maguire@...cle.com>, <pmladek@...e.com>
CC: <linux-kernel@...r.kernel.org>, <bpf@...r.kernel.org>,
Onkarnath <onkarnath.1@...sung.com>
Subject: Re: [PATCH v3 1/3] kallsyms: move kallsyms_show_value() out of
kallsyms.c
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@...sung.com>
> Signed-off-by: Onkarnath <onkarnath.1@...sung.com>
> Signed-off-by: Maninder Singh <maninder1.s@...sung.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
Powered by blists - more mailing lists