[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
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
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 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/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 .