Re: [PATCH v2 5/7] kallsyms: Rename is_kernel() and is_kernel_text()
On 2021/7/29 12:05, Steven Rostedt wrote:
On Thu, 29 Jul 2021 10:00:51 +0800
Kefeng Wang wrote:
On 2021/7/28 23:28, Steven Rostedt wrote:
On Wed, 28 Jul 2021 16:13:18 +0800
Kefeng Wang wrote:
The is_kernel[_text]() function check the address whether or not
in kernel[_text] ranges, also they will check the address whether
or not in gate area, so use better name.
Do you know what a gate area is?
Because I believe gate area is kernel text, so the rename just makes it
redundant and more confusing.
Yes, the gate area(eg, vectors part on ARM32, similar on x86/ia64) is
kernel text.
I want to keep the 'basic' section boundaries check, which only check
the start/end
of sections, all in section.h, could we use 'generic' or 'basic' or
'core' in the naming?
* is_kernel_generic_data() --- come from core_kernel_data() in kernel.h
* is_kernel_generic_text()
The old helper could remain unchanged, any suggestion, thanks.
Because it looks like the check of just being in the range of "_stext"
to "_end" is just an internal helper, why not do what we do all over
the kernel, and just prefix the function with a couple of underscores,
that denote that it's internal?
__is_kernel_text()
OK， thanks for your advise, there's already a __is_kernel_text() in
arch/x86/mm/init_32.c,
I will change it to is_x32_kernel_text() to avoid conflict on x86_32.
Then you have:
static inline int is_kernel_text(unsigned long addr)
{
if (__is_kernel_text(addr))
return 1;
return in_gate_area_no_mm(addr);
}
-- Steve
.
Re: [PATCH v2 5/7] kallsyms: Rename is_kernel() and is_kernel_text()
On Thu, 29 Jul 2021 10:00:51 +0800
Kefeng Wang wrote:
> On 2021/7/28 23:28, Steven Rostedt wrote:
> > On Wed, 28 Jul 2021 16:13:18 +0800
> > Kefeng Wang wrote:
> >
> >> The is_kernel[_text]() function check the address whether or not
> >> in kernel[_text] ranges, also they will check the address whether
> >> or not in gate area, so use better name.
> > Do you know what a gate area is?
> >
> > Because I believe gate area is kernel text, so the rename just makes it
> > redundant and more confusing.
>
> Yes, the gate area(eg, vectors part on ARM32, similar on x86/ia64) is
> kernel text.
>
> I want to keep the 'basic' section boundaries check, which only check
> the start/end
>
> of sections, all in section.h, could we use 'generic' or 'basic' or
> 'core' in the naming?
>
> * is_kernel_generic_data() --- come from core_kernel_data() in kernel.h
> * is_kernel_generic_text()
>
> The old helper could remain unchanged, any suggestion, thanks.
Because it looks like the check of just being in the range of "_stext"
to "_end" is just an internal helper, why not do what we do all over
the kernel, and just prefix the function with a couple of underscores,
that denote that it's internal?
__is_kernel_text()
Then you have:
static inline int is_kernel_text(unsigned long addr)
{
if (__is_kernel_text(addr))
return 1;
return in_gate_area_no_mm(addr);
}
-- Steve
Re: [PATCH v2 5/7] kallsyms: Rename is_kernel() and is_kernel_text()
On 2021/7/28 23:28, Steven Rostedt wrote: On Wed, 28 Jul 2021 16:13:18 +0800 Kefeng Wang wrote: The is_kernel[_text]() function check the address whether or not in kernel[_text] ranges, also they will check the address whether or not in gate area, so use better name. Do you know what a gate area is? Because I believe gate area is kernel text, so the rename just makes it redundant and more confusing. Yes, the gate area(eg, vectors part on ARM32, similar on x86/ia64) is kernel text. I want to keep the 'basic' section boundaries check, which only check the start/end of sections, all in section.h, could we use 'generic' or 'basic' or 'core' in the naming? * is_kernel_generic_data() --- come from core_kernel_data() in kernel.h * is_kernel_generic_text() The old helper could remain unchanged, any suggestion, thanks. -- Steve .
Re: [PATCH v2 5/7] kallsyms: Rename is_kernel() and is_kernel_text()
On Wed, 28 Jul 2021 16:13:18 +0800 Kefeng Wang wrote: > The is_kernel[_text]() function check the address whether or not > in kernel[_text] ranges, also they will check the address whether > or not in gate area, so use better name. Do you know what a gate area is? Because I believe gate area is kernel text, so the rename just makes it redundant and more confusing. -- Steve
[PATCH v2 5/7] kallsyms: Rename is_kernel() and is_kernel_text()
The is_kernel[_text]() function check the address whether or not
in kernel[_text] ranges, also they will check the address whether
or not in gate area, so use better name.
Cc: Alexei Starovoitov
Cc: Daniel Borkmann
Cc: Sami Tolvanen
Cc: Nathan Chancellor
Cc: Arnd Bergmann
Cc: b...@vger.kernel.org
Signed-off-by: Kefeng Wang
---
arch/x86/net/bpf_jit_comp.c | 2 +-
include/linux/kallsyms.h| 8
kernel/cfi.c| 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 333650b9372a..c87d0dd4370d 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -372,7 +372,7 @@ static int __bpf_arch_text_poke(void *ip, enum
bpf_text_poke_type t,
int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
void *old_addr, void *new_addr)
{
- if (!is_kernel_text((long)ip) &&
+ if (!is_kernel_text_or_gate_area((long)ip) &&
!is_bpf_text_address((long)ip))
/* BPF poking in modules is not supported */
return -EINVAL;
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 8a9d329c927c..4f501ac9c2c2 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -24,14 +24,14 @@
struct cred;
struct module;
-static inline int is_kernel_text(unsigned long addr)
+static inline int is_kernel_text_or_gate_area(unsigned long addr)
{
if ((addr >= (unsigned long)_stext && addr < (unsigned long)_etext))
return 1;
return in_gate_area_no_mm(addr);
}
-static inline int is_kernel(unsigned long addr)
+static inline int is_kernel_or_gate_area(unsigned long addr)
{
if (addr >= (unsigned long)_stext && addr < (unsigned long)_end)
return 1;
@@ -41,9 +41,9 @@ static inline int is_kernel(unsigned long addr)
static inline int is_ksym_addr(unsigned long addr)
{
if (IS_ENABLED(CONFIG_KALLSYMS_ALL))
- return is_kernel(addr);
+ return is_kernel_or_gate_area(addr);
- return is_kernel_text(addr) || is_kernel_inittext(addr);
+ return is_kernel_text_or_gate_area(addr) || is_kernel_inittext(addr);
}
static inline void *dereference_symbol_descriptor(void *ptr)
diff --git a/kernel/cfi.c b/kernel/cfi.c
index e17a56639766..e7d90eff4382 100644
--- a/kernel/cfi.c
+++ b/kernel/cfi.c
@@ -282,7 +282,7 @@ static inline cfi_check_fn find_check_fn(unsigned long ptr)
{
cfi_check_fn fn = NULL;
- if (is_kernel_text(ptr))
+ if (is_kernel_text_or_gate_area(ptr))
return __cfi_check;
/*
--
2.26.2
