]> git.ipfire.org Git - people/ms/linux.git/commitdiff
Merge tag 'trace-v6.0-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt...
authorLinus Torvalds <torvalds@linux-foundation.org>
Sun, 21 Aug 2022 21:49:42 +0000 (14:49 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Sun, 21 Aug 2022 21:49:42 +0000 (14:49 -0700)
Pull tracing fixes from Steven Rostedt:
 "Various fixes for tracing:

   - Fix a return value of traceprobe_parse_event_name()

   - Fix NULL pointer dereference from failed ftrace enabling

   - Fix NULL pointer dereference when asking for registers from eprobes

   - Make eprobes consistent with kprobes/uprobes, filters and
     histograms"

* tag 'trace-v6.0-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace:
  tracing: Have filter accept "common_cpu" to be consistent
  tracing/probes: Have kprobes and uprobes use $COMM too
  tracing/eprobes: Have event probes be consistent with kprobes and uprobes
  tracing/eprobes: Fix reading of string fields
  tracing/eprobes: Do not hardcode $comm as a string
  tracing/eprobes: Do not allow eprobes to use $stack, or % for regs
  ftrace: Fix NULL pointer dereference in is_ftrace_trampoline when ftrace is dead
  tracing/perf: Fix double put of trace event when init fails
  tracing: React to error return from traceprobe_parse_event_name()

1  2 
kernel/trace/ftrace.c

diff --combined kernel/trace/ftrace.c
index bc921a3f7ea894f975238c7d0319f1cbd94ec9c5,4baa99363b166cdc741c3e0e70e111a603f8310c..126c769d36c3cf48cf46bb495b96cf834662b5cc
@@@ -1861,8 -1861,6 +1861,8 @@@ static void ftrace_hash_rec_enable_modi
        ftrace_hash_rec_update_modify(ops, filter_hash, 1);
  }
  
 +static bool ops_references_ip(struct ftrace_ops *ops, unsigned long ip);
 +
  /*
   * Try to update IPMODIFY flag on each ftrace_rec. Return 0 if it is OK
   * or no-needed to update, -EBUSY if it detects a conflict of the flag
   *  - If the hash is NULL, it hits all recs (if IPMODIFY is set, this is rejected)
   *  - If the hash is EMPTY_HASH, it hits nothing
   *  - Anything else hits the recs which match the hash entries.
 + *
 + * DIRECT ops does not have IPMODIFY flag, but we still need to check it
 + * against functions with FTRACE_FL_IPMODIFY. If there is any overlap, call
 + * ops_func(SHARE_IPMODIFY_SELF) to make sure current ops can share with
 + * IPMODIFY. If ops_func(SHARE_IPMODIFY_SELF) returns non-zero, propagate
 + * the return value to the caller and eventually to the owner of the DIRECT
 + * ops.
   */
  static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
                                         struct ftrace_hash *old_hash,
        struct ftrace_page *pg;
        struct dyn_ftrace *rec, *end = NULL;
        int in_old, in_new;
 +      bool is_ipmodify, is_direct;
  
        /* Only update if the ops has been registered */
        if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
                return 0;
  
 -      if (!(ops->flags & FTRACE_OPS_FL_IPMODIFY))
 +      is_ipmodify = ops->flags & FTRACE_OPS_FL_IPMODIFY;
 +      is_direct = ops->flags & FTRACE_OPS_FL_DIRECT;
 +
 +      /* neither IPMODIFY nor DIRECT, skip */
 +      if (!is_ipmodify && !is_direct)
 +              return 0;
 +
 +      if (WARN_ON_ONCE(is_ipmodify && is_direct))
                return 0;
  
        /*
 -       * Since the IPMODIFY is a very address sensitive action, we do not
 -       * allow ftrace_ops to set all functions to new hash.
 +       * Since the IPMODIFY and DIRECT are very address sensitive
 +       * actions, we do not allow ftrace_ops to set all functions to new
 +       * hash.
         */
        if (!new_hash || !old_hash)
                return -EINVAL;
                        continue;
  
                if (in_new) {
 -                      /* New entries must ensure no others are using it */
 -                      if (rec->flags & FTRACE_FL_IPMODIFY)
 -                              goto rollback;
 -                      rec->flags |= FTRACE_FL_IPMODIFY;
 -              } else /* Removed entry */
 +                      if (rec->flags & FTRACE_FL_IPMODIFY) {
 +                              int ret;
 +
 +                              /* Cannot have two ipmodify on same rec */
 +                              if (is_ipmodify)
 +                                      goto rollback;
 +
 +                              FTRACE_WARN_ON(rec->flags & FTRACE_FL_DIRECT);
 +
 +                              /*
 +                               * Another ops with IPMODIFY is already
 +                               * attached. We are now attaching a direct
 +                               * ops. Run SHARE_IPMODIFY_SELF, to check
 +                               * whether sharing is supported.
 +                               */
 +                              if (!ops->ops_func)
 +                                      return -EBUSY;
 +                              ret = ops->ops_func(ops, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_SELF);
 +                              if (ret)
 +                                      return ret;
 +                      } else if (is_ipmodify) {
 +                              rec->flags |= FTRACE_FL_IPMODIFY;
 +                      }
 +              } else if (is_ipmodify) {
                        rec->flags &= ~FTRACE_FL_IPMODIFY;
 +              }
        } while_for_each_ftrace_rec();
  
        return 0;
@@@ -2492,7 -2454,8 +2492,7 @@@ static void call_direct_funcs(unsigned 
  
  struct ftrace_ops direct_ops = {
        .func           = call_direct_funcs,
 -      .flags          = FTRACE_OPS_FL_IPMODIFY
 -                        | FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS
 +      .flags          = FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS
                          | FTRACE_OPS_FL_PERMANENT,
        /*
         * By declaring the main trampoline as this trampoline
@@@ -2974,6 -2937,16 +2974,16 @@@ int ftrace_startup(struct ftrace_ops *o
  
        ftrace_startup_enable(command);
  
+       /*
+        * If ftrace is in an undefined state, we just remove ops from list
+        * to prevent the NULL pointer, instead of totally rolling it back and
+        * free trampoline, because those actions could cause further damage.
+        */
+       if (unlikely(ftrace_disabled)) {
+               __unregister_ftrace_function(ops);
+               return -ENODEV;
+       }
        ops->flags &= ~FTRACE_OPS_FL_ADDING;
  
        return 0;
@@@ -3109,14 -3082,14 +3119,14 @@@ static inline int ops_traces_mod(struc
  }
  
  /*
 - * Check if the current ops references the record.
 + * Check if the current ops references the given ip.
   *
   * If the ops traces all functions, then it was already accounted for.
   * If the ops does not trace the current record function, skip it.
   * If the ops ignores the function via notrace filter, skip it.
   */
 -static inline bool
 -ops_references_rec(struct ftrace_ops *ops, struct dyn_ftrace *rec)
 +static bool
 +ops_references_ip(struct ftrace_ops *ops, unsigned long ip)
  {
        /* If ops isn't enabled, ignore it */
        if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
  
        /* The function must be in the filter */
        if (!ftrace_hash_empty(ops->func_hash->filter_hash) &&
 -          !__ftrace_lookup_ip(ops->func_hash->filter_hash, rec->ip))
 +          !__ftrace_lookup_ip(ops->func_hash->filter_hash, ip))
                return false;
  
        /* If in notrace hash, we ignore it too */
 -      if (ftrace_lookup_ip(ops->func_hash->notrace_hash, rec->ip))
 +      if (ftrace_lookup_ip(ops->func_hash->notrace_hash, ip))
                return false;
  
        return true;
  }
  
 +/*
 + * Check if the current ops references the record.
 + *
 + * If the ops traces all functions, then it was already accounted for.
 + * If the ops does not trace the current record function, skip it.
 + * If the ops ignores the function via notrace filter, skip it.
 + */
 +static bool
 +ops_references_rec(struct ftrace_ops *ops, struct dyn_ftrace *rec)
 +{
 +      return ops_references_ip(ops, rec->ip);
 +}
 +
  static int ftrace_update_code(struct module *mod, struct ftrace_page *new_pgs)
  {
        bool init_nop = ftrace_need_init_nop();
@@@ -5265,8 -5225,6 +5275,8 @@@ static struct ftrace_direct_func *ftrac
        return direct;
  }
  
 +static int register_ftrace_function_nolock(struct ftrace_ops *ops);
 +
  /**
   * register_ftrace_direct - Call a custom trampoline directly
   * @ip: The address of the nop at the beginning of a function
@@@ -5338,7 -5296,7 +5348,7 @@@ int register_ftrace_direct(unsigned lon
        ret = ftrace_set_filter_ip(&direct_ops, ip, 0, 0);
  
        if (!ret && !(direct_ops.flags & FTRACE_OPS_FL_ENABLED)) {
 -              ret = register_ftrace_function(&direct_ops);
 +              ret = register_ftrace_function_nolock(&direct_ops);
                if (ret)
                        ftrace_set_filter_ip(&direct_ops, ip, 1, 0);
        }
@@@ -5597,7 -5555,8 +5607,7 @@@ int modify_ftrace_direct(unsigned long 
  }
  EXPORT_SYMBOL_GPL(modify_ftrace_direct);
  
 -#define MULTI_FLAGS (FTRACE_OPS_FL_IPMODIFY | FTRACE_OPS_FL_DIRECT | \
 -                   FTRACE_OPS_FL_SAVE_REGS)
 +#define MULTI_FLAGS (FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS)
  
  static int check_direct_multi(struct ftrace_ops *ops)
  {
@@@ -5690,7 -5649,7 +5700,7 @@@ int register_ftrace_direct_multi(struc
        ops->flags = MULTI_FLAGS;
        ops->trampoline = FTRACE_REGS_ADDR;
  
 -      err = register_ftrace_function(ops);
 +      err = register_ftrace_function_nolock(ops);
  
   out_remove:
        if (err)
@@@ -5742,8 -5701,22 +5752,8 @@@ int unregister_ftrace_direct_multi(stru
  }
  EXPORT_SYMBOL_GPL(unregister_ftrace_direct_multi);
  
 -/**
 - * modify_ftrace_direct_multi - Modify an existing direct 'multi' call
 - * to call something else
 - * @ops: The address of the struct ftrace_ops object
 - * @addr: The address of the new trampoline to call at @ops functions
 - *
 - * This is used to unregister currently registered direct caller and
 - * register new one @addr on functions registered in @ops object.
 - *
 - * Note there's window between ftrace_shutdown and ftrace_startup calls
 - * where there will be no callbacks called.
 - *
 - * Returns: zero on success. Non zero on error, which includes:
 - *  -EINVAL - The @ops object was not properly registered.
 - */
 -int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
 +static int
 +__modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
  {
        struct ftrace_hash *hash;
        struct ftrace_func_entry *entry, *iter;
        int i, size;
        int err;
  
 -      if (check_direct_multi(ops))
 -              return -EINVAL;
 -      if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
 -              return -EINVAL;
 -
 -      mutex_lock(&direct_mutex);
 +      lockdep_assert_held_once(&direct_mutex);
  
        /* Enable the tmp_ops to have the same functions as the direct ops */
        ftrace_ops_init(&tmp_ops);
        tmp_ops.func_hash = ops->func_hash;
  
 -      err = register_ftrace_function(&tmp_ops);
 +      err = register_ftrace_function_nolock(&tmp_ops);
        if (err)
 -              goto out_direct;
 +              return err;
  
        /*
         * Now the ftrace_ops_list_func() is called to do the direct callers.
        /* Removing the tmp_ops will add the updated direct callers to the functions */
        unregister_ftrace_function(&tmp_ops);
  
 - out_direct:
 +      return err;
 +}
 +
 +/**
 + * modify_ftrace_direct_multi_nolock - Modify an existing direct 'multi' call
 + * to call something else
 + * @ops: The address of the struct ftrace_ops object
 + * @addr: The address of the new trampoline to call at @ops functions
 + *
 + * This is used to unregister currently registered direct caller and
 + * register new one @addr on functions registered in @ops object.
 + *
 + * Note there's window between ftrace_shutdown and ftrace_startup calls
 + * where there will be no callbacks called.
 + *
 + * Caller should already have direct_mutex locked, so we don't lock
 + * direct_mutex here.
 + *
 + * Returns: zero on success. Non zero on error, which includes:
 + *  -EINVAL - The @ops object was not properly registered.
 + */
 +int modify_ftrace_direct_multi_nolock(struct ftrace_ops *ops, unsigned long addr)
 +{
 +      if (check_direct_multi(ops))
 +              return -EINVAL;
 +      if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
 +              return -EINVAL;
 +
 +      return __modify_ftrace_direct_multi(ops, addr);
 +}
 +EXPORT_SYMBOL_GPL(modify_ftrace_direct_multi_nolock);
 +
 +/**
 + * modify_ftrace_direct_multi - Modify an existing direct 'multi' call
 + * to call something else
 + * @ops: The address of the struct ftrace_ops object
 + * @addr: The address of the new trampoline to call at @ops functions
 + *
 + * This is used to unregister currently registered direct caller and
 + * register new one @addr on functions registered in @ops object.
 + *
 + * Note there's window between ftrace_shutdown and ftrace_startup calls
 + * where there will be no callbacks called.
 + *
 + * Returns: zero on success. Non zero on error, which includes:
 + *  -EINVAL - The @ops object was not properly registered.
 + */
 +int modify_ftrace_direct_multi(struct ftrace_ops *ops, unsigned long addr)
 +{
 +      int err;
 +
 +      if (check_direct_multi(ops))
 +              return -EINVAL;
 +      if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
 +              return -EINVAL;
 +
 +      mutex_lock(&direct_mutex);
 +      err = __modify_ftrace_direct_multi(ops, addr);
        mutex_unlock(&direct_mutex);
        return err;
  }
@@@ -8054,143 -7975,6 +8064,143 @@@ int ftrace_is_dead(void
        return ftrace_disabled;
  }
  
 +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 +/*
 + * When registering ftrace_ops with IPMODIFY, it is necessary to make sure
 + * it doesn't conflict with any direct ftrace_ops. If there is existing
 + * direct ftrace_ops on a kernel function being patched, call
 + * FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER on it to enable sharing.
 + *
 + * @ops:     ftrace_ops being registered.
 + *
 + * Returns:
 + *         0 on success;
 + *         Negative on failure.
 + */
 +static int prepare_direct_functions_for_ipmodify(struct ftrace_ops *ops)
 +{
 +      struct ftrace_func_entry *entry;
 +      struct ftrace_hash *hash;
 +      struct ftrace_ops *op;
 +      int size, i, ret;
 +
 +      lockdep_assert_held_once(&direct_mutex);
 +
 +      if (!(ops->flags & FTRACE_OPS_FL_IPMODIFY))
 +              return 0;
 +
 +      hash = ops->func_hash->filter_hash;
 +      size = 1 << hash->size_bits;
 +      for (i = 0; i < size; i++) {
 +              hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
 +                      unsigned long ip = entry->ip;
 +                      bool found_op = false;
 +
 +                      mutex_lock(&ftrace_lock);
 +                      do_for_each_ftrace_op(op, ftrace_ops_list) {
 +                              if (!(op->flags & FTRACE_OPS_FL_DIRECT))
 +                                      continue;
 +                              if (ops_references_ip(op, ip)) {
 +                                      found_op = true;
 +                                      break;
 +                              }
 +                      } while_for_each_ftrace_op(op);
 +                      mutex_unlock(&ftrace_lock);
 +
 +                      if (found_op) {
 +                              if (!op->ops_func)
 +                                      return -EBUSY;
 +
 +                              ret = op->ops_func(op, FTRACE_OPS_CMD_ENABLE_SHARE_IPMODIFY_PEER);
 +                              if (ret)
 +                                      return ret;
 +                      }
 +              }
 +      }
 +
 +      return 0;
 +}
 +
 +/*
 + * Similar to prepare_direct_functions_for_ipmodify, clean up after ops
 + * with IPMODIFY is unregistered. The cleanup is optional for most DIRECT
 + * ops.
 + */
 +static void cleanup_direct_functions_after_ipmodify(struct ftrace_ops *ops)
 +{
 +      struct ftrace_func_entry *entry;
 +      struct ftrace_hash *hash;
 +      struct ftrace_ops *op;
 +      int size, i;
 +
 +      if (!(ops->flags & FTRACE_OPS_FL_IPMODIFY))
 +              return;
 +
 +      mutex_lock(&direct_mutex);
 +
 +      hash = ops->func_hash->filter_hash;
 +      size = 1 << hash->size_bits;
 +      for (i = 0; i < size; i++) {
 +              hlist_for_each_entry(entry, &hash->buckets[i], hlist) {
 +                      unsigned long ip = entry->ip;
 +                      bool found_op = false;
 +
 +                      mutex_lock(&ftrace_lock);
 +                      do_for_each_ftrace_op(op, ftrace_ops_list) {
 +                              if (!(op->flags & FTRACE_OPS_FL_DIRECT))
 +                                      continue;
 +                              if (ops_references_ip(op, ip)) {
 +                                      found_op = true;
 +                                      break;
 +                              }
 +                      } while_for_each_ftrace_op(op);
 +                      mutex_unlock(&ftrace_lock);
 +
 +                      /* The cleanup is optional, ignore any errors */
 +                      if (found_op && op->ops_func)
 +                              op->ops_func(op, FTRACE_OPS_CMD_DISABLE_SHARE_IPMODIFY_PEER);
 +              }
 +      }
 +      mutex_unlock(&direct_mutex);
 +}
 +
 +#define lock_direct_mutex()   mutex_lock(&direct_mutex)
 +#define unlock_direct_mutex() mutex_unlock(&direct_mutex)
 +
 +#else  /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
 +
 +static int prepare_direct_functions_for_ipmodify(struct ftrace_ops *ops)
 +{
 +      return 0;
 +}
 +
 +static void cleanup_direct_functions_after_ipmodify(struct ftrace_ops *ops)
 +{
 +}
 +
 +#define lock_direct_mutex()   do { } while (0)
 +#define unlock_direct_mutex() do { } while (0)
 +
 +#endif  /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
 +
 +/*
 + * Similar to register_ftrace_function, except we don't lock direct_mutex.
 + */
 +static int register_ftrace_function_nolock(struct ftrace_ops *ops)
 +{
 +      int ret;
 +
 +      ftrace_ops_init(ops);
 +
 +      mutex_lock(&ftrace_lock);
 +
 +      ret = ftrace_startup(ops, 0);
 +
 +      mutex_unlock(&ftrace_lock);
 +
 +      return ret;
 +}
 +
  /**
   * register_ftrace_function - register a function for profiling
   * @ops:      ops structure that holds the function for profiling.
@@@ -8206,15 -7990,14 +8216,15 @@@ int register_ftrace_function(struct ftr
  {
        int ret;
  
 -      ftrace_ops_init(ops);
 -
 -      mutex_lock(&ftrace_lock);
 -
 -      ret = ftrace_startup(ops, 0);
 +      lock_direct_mutex();
 +      ret = prepare_direct_functions_for_ipmodify(ops);
 +      if (ret < 0)
 +              goto out_unlock;
  
 -      mutex_unlock(&ftrace_lock);
 +      ret = register_ftrace_function_nolock(ops);
  
 +out_unlock:
 +      unlock_direct_mutex();
        return ret;
  }
  EXPORT_SYMBOL_GPL(register_ftrace_function);
@@@ -8233,7 -8016,6 +8243,7 @@@ int unregister_ftrace_function(struct f
        ret = ftrace_shutdown(ops, 0);
        mutex_unlock(&ftrace_lock);
  
 +      cleanup_direct_functions_after_ipmodify(ops);
        return ret;
  }
  EXPORT_SYMBOL_GPL(unregister_ftrace_function);