On Mon, 22 Apr 2024 12:44:35 +0300
Mike Rapoport <r...@kernel.org> wrote:

> From: "Mike Rapoport (IBM)" <r...@kernel.org>
> 
> kprobes depended on CONFIG_MODULES because it has to allocate memory for
> code.
> 
> Since code allocations are now implemented with execmem, kprobes can be
> enabled in non-modular kernels.
> 
> Add #ifdef CONFIG_MODULE guards for the code dealing with kprobes inside
> modules, make CONFIG_KPROBES select CONFIG_EXECMEM and drop the
> dependency of CONFIG_KPROBES on CONFIG_MODULES.

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhira...@kernel.org>

Thank you!

> 
> Signed-off-by: Mike Rapoport (IBM) <r...@kernel.org>
> ---
>  arch/Kconfig                |  2 +-
>  include/linux/module.h      |  9 ++++++
>  kernel/kprobes.c            | 55 +++++++++++++++++++++++--------------
>  kernel/trace/trace_kprobe.c | 20 +++++++++++++-
>  4 files changed, 63 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 7006f71f0110..a48ce6a488b3 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -52,9 +52,9 @@ config GENERIC_ENTRY
>  
>  config KPROBES
>       bool "Kprobes"
> -     depends on MODULES
>       depends on HAVE_KPROBES
>       select KALLSYMS
> +     select EXECMEM
>       select TASKS_RCU if PREEMPTION
>       help
>         Kprobes allows you to trap at almost any kernel address and
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 1153b0d99a80..ffa1c603163c 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -605,6 +605,11 @@ static inline bool module_is_live(struct module *mod)
>       return mod->state != MODULE_STATE_GOING;
>  }
>  
> +static inline bool module_is_coming(struct module *mod)
> +{
> +        return mod->state == MODULE_STATE_COMING;
> +}
> +
>  struct module *__module_text_address(unsigned long addr);
>  struct module *__module_address(unsigned long addr);
>  bool is_module_address(unsigned long addr);
> @@ -857,6 +862,10 @@ void *dereference_module_function_descriptor(struct 
> module *mod, void *ptr)
>       return ptr;
>  }
>  
> +static inline bool module_is_coming(struct module *mod)
> +{
> +     return false;
> +}
>  #endif /* CONFIG_MODULES */
>  
>  #ifdef CONFIG_SYSFS
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ddd7cdc16edf..ca2c6cbd42d2 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1588,7 +1588,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
>       }
>  
>       /* Get module refcount and reject __init functions for loaded modules. 
> */
> -     if (*probed_mod) {
> +     if (IS_ENABLED(CONFIG_MODULES) && *probed_mod) {
>               /*
>                * We must hold a refcount of the probed module while updating
>                * its code to prohibit unexpected unloading.
> @@ -1603,12 +1603,13 @@ static int check_kprobe_address_safe(struct kprobe *p,
>                * kprobes in there.
>                */
>               if (within_module_init((unsigned long)p->addr, *probed_mod) &&
> -                 (*probed_mod)->state != MODULE_STATE_COMING) {
> +                 !module_is_coming(*probed_mod)) {
>                       module_put(*probed_mod);
>                       *probed_mod = NULL;
>                       ret = -ENOENT;
>               }
>       }
> +
>  out:
>       preempt_enable();
>       jump_label_unlock();
> @@ -2488,24 +2489,6 @@ int kprobe_add_area_blacklist(unsigned long start, 
> unsigned long end)
>       return 0;
>  }
>  
> -/* Remove all symbols in given area from kprobe blacklist */
> -static void kprobe_remove_area_blacklist(unsigned long start, unsigned long 
> end)
> -{
> -     struct kprobe_blacklist_entry *ent, *n;
> -
> -     list_for_each_entry_safe(ent, n, &kprobe_blacklist, list) {
> -             if (ent->start_addr < start || ent->start_addr >= end)
> -                     continue;
> -             list_del(&ent->list);
> -             kfree(ent);
> -     }
> -}
> -
> -static void kprobe_remove_ksym_blacklist(unsigned long entry)
> -{
> -     kprobe_remove_area_blacklist(entry, entry + 1);
> -}
> -
>  int __weak arch_kprobe_get_kallsym(unsigned int *symnum, unsigned long 
> *value,
>                                  char *type, char *sym)
>  {
> @@ -2570,6 +2553,25 @@ static int __init populate_kprobe_blacklist(unsigned 
> long *start,
>       return ret ? : arch_populate_kprobe_blacklist();
>  }
>  
> +#ifdef CONFIG_MODULES
> +/* Remove all symbols in given area from kprobe blacklist */
> +static void kprobe_remove_area_blacklist(unsigned long start, unsigned long 
> end)
> +{
> +     struct kprobe_blacklist_entry *ent, *n;
> +
> +     list_for_each_entry_safe(ent, n, &kprobe_blacklist, list) {
> +             if (ent->start_addr < start || ent->start_addr >= end)
> +                     continue;
> +             list_del(&ent->list);
> +             kfree(ent);
> +     }
> +}
> +
> +static void kprobe_remove_ksym_blacklist(unsigned long entry)
> +{
> +     kprobe_remove_area_blacklist(entry, entry + 1);
> +}
> +
>  static void add_module_kprobe_blacklist(struct module *mod)
>  {
>       unsigned long start, end;
> @@ -2672,6 +2674,17 @@ static struct notifier_block kprobe_module_nb = {
>       .priority = 0
>  };
>  
> +static int kprobe_register_module_notifier(void)
> +{
> +     return register_module_notifier(&kprobe_module_nb);
> +}
> +#else
> +static int kprobe_register_module_notifier(void)
> +{
> +     return 0;
> +}
> +#endif /* CONFIG_MODULES */
> +
>  void kprobe_free_init_mem(void)
>  {
>       void *start = (void *)(&__init_begin);
> @@ -2731,7 +2744,7 @@ static int __init init_kprobes(void)
>       if (!err)
>               err = register_die_notifier(&kprobe_exceptions_nb);
>       if (!err)
> -             err = register_module_notifier(&kprobe_module_nb);
> +             err = kprobe_register_module_notifier();
>  
>       kprobes_initialized = (err == 0);
>       kprobe_sysctls_init();
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 14099cc17fc9..2cb2a3951b4f 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -111,6 +111,7 @@ static nokprobe_inline bool 
> trace_kprobe_within_module(struct trace_kprobe *tk,
>       return strncmp(module_name(mod), name, len) == 0 && name[len] == ':';
>  }
>  
> +#ifdef CONFIG_MODULES
>  static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe 
> *tk)
>  {
>       char *p;
> @@ -129,6 +130,12 @@ static nokprobe_inline bool 
> trace_kprobe_module_exist(struct trace_kprobe *tk)
>  
>       return ret;
>  }
> +#else
> +static inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
> +{
> +     return false;
> +}
> +#endif
>  
>  static bool trace_kprobe_is_busy(struct dyn_event *ev)
>  {
> @@ -670,6 +677,7 @@ static int register_trace_kprobe(struct trace_kprobe *tk)
>       return ret;
>  }
>  
> +#ifdef CONFIG_MODULES
>  /* Module notifier call back, checking event on the module */
>  static int trace_kprobe_module_callback(struct notifier_block *nb,
>                                      unsigned long val, void *data)
> @@ -704,6 +712,16 @@ static struct notifier_block trace_kprobe_module_nb = {
>       .notifier_call = trace_kprobe_module_callback,
>       .priority = 1   /* Invoked after kprobe module callback */
>  };
> +static int trace_kprobe_register_module_notifier(void)
> +{
> +     return register_module_notifier(&trace_kprobe_module_nb);
> +}
> +#else
> +static int trace_kprobe_register_module_notifier(void)
> +{
> +     return 0;
> +}
> +#endif /* CONFIG_MODULES */
>  
>  static int count_symbols(void *data, unsigned long unused)
>  {
> @@ -1933,7 +1951,7 @@ static __init int init_kprobe_trace_early(void)
>       if (ret)
>               return ret;
>  
> -     if (register_module_notifier(&trace_kprobe_module_nb))
> +     if (trace_kprobe_register_module_notifier())
>               return -EINVAL;
>  
>       return 0;
> -- 
> 2.43.0
> 


-- 
Masami Hiramatsu (Google) <mhira...@kernel.org>

Reply via email to