[v4,1/3] kallsyms: move kallsyms_show_value() out of kallsyms.c

Commit Message

Maninder Singh June 6, 2023, 4:28 a.m. UTC

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 ksyms_common.c.

With this patch code is just moved to new file
and no functional change.

Co-developed-by: Onkarnath <[e-mail]>
Signed-off-by: Onkarnath <[e-mail]>
Signed-off-by: Maninder Singh <[e-mail]>
---
earlier conversations:(then it has dependancy on other change, but that
was stashed from linux-next, now it can be pushed)
https://lore.kernel.org/lkml/202205111525.92B1C597@keescook/T/
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
v3 -> v4: patch order changed, file name changed form knosyms -> ksyms_common
and copyright header modified.

 kernel/Makefile       |  2 +-
 kernel/kallsyms.c     | 35 ---------------------------------
 kernel/ksyms_common.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 36 deletions(-)
 create mode 100644 kernel/ksyms_common.c

Comments

Zhen Lei June 6, 2023, 11:23 a.m. UTC | #1

On 2023/6/6 12:28, 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 ksyms_common.c.
> 
> With this patch code is just moved to new file
> and no functional change.
> 
> Co-developed-by: Onkarnath <[e-mail]>
> Signed-off-by: Onkarnath <[e-mail]>
> Signed-off-by: Maninder Singh <[e-mail]>
> ---
> earlier conversations:(then it has dependancy on other change, but that
> was stashed from linux-next, now it can be pushed)
> https://lore.kernel.org/lkml/202205111525.92B1C597@keescook/T/
> 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
> v3 -> v4: patch order changed, file name changed form knosyms -> ksyms_common
> and copyright header modified.
> 
>  kernel/Makefile       |  2 +-
>  kernel/kallsyms.c     | 35 ---------------------------------
>  kernel/ksyms_common.c | 45 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 46 insertions(+), 36 deletions(-)
>  create mode 100644 kernel/ksyms_common.c
> 
> diff --git a/kernel/Makefile b/kernel/Makefile
> index f9e3fd9195d9..3947122d618b 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 ksyms_common.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/ksyms_common.c b/kernel/ksyms_common.c
> new file mode 100644
> index 000000000000..e776f12f0f5a
> --- /dev/null
> +++ b/kernel/ksyms_common.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0

Keep it the same as kernel/kallsyms.c. GPL-2.0-only

Sorry, I didn't think of that last time.
Otherwise,

Reviewed-by: Zhen Lei <[e-mail]>

> +/*
> + * ksyms_common.c: A split of kernel/kallsyms.c
> + * Contains a 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
>

Patch

diff --git a/kernel/Makefile b/kernel/Makefile
index f9e3fd9195d9..3947122d618b 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 ksyms_common.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/ksyms_common.c b/kernel/ksyms_common.c
new file mode 100644
index 000000000000..e776f12f0f5a
--- /dev/null
+++ b/kernel/ksyms_common.c
@@ -0,0 +1,45 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ksyms_common.c: A split of kernel/kallsyms.c
+ * Contains a 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