Re: [PATCH v2 03/12] libtracefs: New kprobes APIs

[Yordan Karadzhov <y.karadz@gmail.com>]

On 1.11.21 г. 11:08, Tzvetomir Stoyanov (VMware) wrote:
> In order to be consistent with the other APIs, a new set of kprobe APIs
> is introduced:
>   tracefs_kprobe_alloc();
>   tracefs_kretprobe_alloc();
>   tracefs_kprobe_create();
>   tracefs_kprobe_destroy();
>   tracefs_kprobe_free();
> These APIs work with kprobe context, represented by the tracefs_dynevent
> structure.
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>   include/tracefs.h     |   8 ++
>   src/tracefs-kprobes.c | 168 ++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 176 insertions(+)
> 
> diff --git a/include/tracefs.h b/include/tracefs.h
> index 4e721eb..85185ce 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -247,6 +247,14 @@ enum tracefs_kprobe_type {
>   	TRACEFS_KRETPROBE,
>   };
>   
> +struct tracefs_dynevent *
> +tracefs_kprobe_alloc(const char *system, const char *event, const char *addr, const char *format);
> +struct tracefs_dynevent *
> +tracefs_kretprobe_alloc(const char *system, const char *event,
> +			const char *addr, const char *format, int max);
> +int tracefs_kprobe_create(struct tracefs_dynevent *kprobe);
> +int tracefs_kprobe_destroy(struct tracefs_dynevent *kprobe, bool force);
> +void tracefs_kprobe_free(struct tracefs_dynevent *kprobe);
>   int tracefs_kprobe_raw(const char *system, const char *event,
>   		       const char *addr, const char *format);
>   int tracefs_kretprobe_raw(const char *system, const char *event,
> diff --git a/src/tracefs-kprobes.c b/src/tracefs-kprobes.c
> index d4c5f9e..906f914 100644
> --- a/src/tracefs-kprobes.c
> +++ b/src/tracefs-kprobes.c
> @@ -20,6 +20,134 @@
>   #define KPROBE_EVENTS "kprobe_events"
>   #define KPROBE_DEFAULT_GROUP "kprobes"
>   
> +static struct tracefs_dynevent *
> +kprobe_alloc(enum trace_dynevent_type type, const char *system, const char *event,
> +	     const char *addr, const char *format)
> +{
> +	struct tracefs_dynevent *kp;
> +	const char *sys = system;
> +	const char *ename = event;
> +	char *tmp;
> +
> +	if (!addr) {
> +		errno = EBADMSG;
> +		return NULL;
> +	}
> +	if (!sys)
> +		sys = KPROBE_DEFAULT_GROUP;
> +
> +	if (!event) {
> +		ename = strdup(addr);
> +		if (!ename)
> +			return NULL;
> +		tmp = strchr(ename, ':');
> +		if (tmp)
> +			*tmp = '\0';
> +	}
> +
> +	kp = dynevent_alloc(type, sys, ename, addr, format);
> +	if (!event)
> +		free((char *)ename);
> +
> +	return kp;
> +}
> +
> +
> +/**
> + * tracefs_kprobe_alloc - Allocate new kprobe context
> + * @system: The system name (NULL for the default kprobes)
> + * @event: The event to create (NULL to use @addr for the event)
> + * @addr: The function and offset (or address) to insert the probe
> + * @format: The format string to define the probe.
> + *
> + * Allocate a kprobe context that will be in the @system group (or kprobes if
> + * @system is NULL). Have the name of @event (or @addr if @event is
> + * NULL). Will be inserted to @addr (function name, with or without
> + * offset, or a address). And the @format will define the format
> + * of the kprobe. See the Linux documentation file under:
> + * Documentation/trace/kprobetrace.rst
> + * The kprobe is not created in the system.
> + *
> + * Return a pointer to a kprobe context on success, or NULL on error.
> + * The returned pointer must be freed with tracefs_kprobe_free()
> + *
> + * errno will be set to EBADMSG if addr is NULL.
> + */
> +struct tracefs_dynevent *
> +tracefs_kprobe_alloc(const char *system, const char *event, const char *addr, const char *format)
> +
> +{
> +	return kprobe_alloc(TRACE_DYNEVENT_KPROBE, system, event, addr, format);
> +}
> +
> +/**
> + * tracefs_kretprobe_alloc - Allocate new kretprobe context
> + * @system: The system name (NULL for the default kprobes)
> + * @event: The event to create (NULL to use @addr for the event)
> + * @addr: The function and offset (or address) to insert the retprobe
> + * @format: The format string to define the retprobe.
> + *

@max is not documented.

> + * Allocate a kretprobe that will be in the @system group (or kprobes if
> + * @system is NULL). Have the name of @event (or @addr if @event is
> + * NULL). Will be inserted to @addr (function name, with or without
> + * offset, or a address). And the @format will define the raw format
> + * of the kprobe. See the Linux documentation file under:
> + * Documentation/trace/kprobetrace.rst
> + * The kretprobe is not created in the system.
> + *
> + * Return a pointer to a kprobe context on success, or NULL on error.
> + * The returned pointer must be freed with tracefs_kprobe_free()
> + *
> + * errno will be set to EBADMSG if addr is NULL.
> + */
> +struct tracefs_dynevent *
> +tracefs_kretprobe_alloc(const char *system, const char *event,
> +			const char *addr, const char *format, int max)
> +{
> +	struct tracefs_dynevent *kp;
> +	int ret;
> +
> +	kp = kprobe_alloc(TRACE_DYNEVENT_KRETPROBE, system, event, addr, format);
> +	if (!kp)
> +		return NULL;
> +	if (max) {
> +		free(kp->prefix);
> +		kp->prefix = NULL;
> +		ret = asprintf(&kp->prefix, "r%d:", max);
> +		if (ret < 0)
> +			goto error;
> +	}
> +
> +	return kp;
> +error:
> +	dynevent_free(kp);
> +	return NULL;
> +}
> +
> +/**
> + * tracefs_kprobe_create - Create a kprobe or kretprobe in the system
> + * @kprobe: Pointer to a kprobe context, describing the probe
> + *
> + * Return 0 on success, or -1 on error.
> + */
> +int tracefs_kprobe_create(struct tracefs_dynevent *kprobe)
> +{
> +	return dynevent_create(kprobe);
> +}
> +

hmm, what will happen if if you have a code like this:


struct tracefs_dynevent *synth = tracefs_synth_alloc(...)

int ret = tracefs_kprobe_create(synth);


Are you just giving several additional fake names to the same function ('dynevent_create()')?


> +/**
> + * tracefs_kprobe_free - Free a kprobe context
> + * @kprobe: Pointer to a kprobe context
> + *
> + * The kprobe/kretprobe, described by this context, is not
> + * removed from the system by this API. It only frees the memory.
> + */
> +void tracefs_kprobe_free(struct tracefs_dynevent *kprobe)
> +{
> +	dynevent_free(kprobe);
> +}
> +

The same argument applys here.

Thanks!
Yordan

> +
>   static int insert_kprobe(const char *type, const char *system,
>   			 const char *event, const char *addr,
>   			 const char *format)
> @@ -474,3 +602,43 @@ int tracefs_kprobe_clear_probe(const char *system, const char *event, bool force
>   
>   	return ret < 0 ? -1 : 0;
>   }
> +
> +/**
> + * tracefs_kprobe_destroy - Remove a kprobe or kretprobe from the system
> + * @kprobe: A kprobe context, describing the kprobe that will be deleted.
> + *	    If NULL, all kprobes and kretprobes in the system will be deleted
> + * @force: Will attempt to disable all kprobe events and clear them
> + *
> + * The kprobe/kretprobe context is not freed by this API.
> + * It only removes the probe from the system.
> + *
> + * Return 0 on success, or -1 on error.
> + */
> +int tracefs_kprobe_destroy(struct tracefs_dynevent *kprobe, bool force)
> +{
> +	char **instance_list;
> +	int ret;
> +
> +	if (!kprobe) {
> +		if (tracefs_instance_file_clear(NULL, KPROBE_EVENTS) == 0)
> +			return 0;
> +		if (!force)
> +			return -1;
> +		/* Attempt to disable all kprobe events */
> +		return kprobe_clear_probes(NULL, force);
> +	}
> +
> +	/*
> +	 * Since we know we are disabling a specific event, try
> +	 * to disable it first before clearing it.
> +	 */
> +	if (force) {
> +		instance_list = tracefs_instances(NULL);
> +		disable_events(kprobe->system, kprobe->event, instance_list);
> +		tracefs_list_free(instance_list);
> +	}
> +
> +	ret = dynevent_destroy(kprobe);
> +
> +	return ret < 0 ? -1 : 0;
> +}
>