On Sat, 16 May 2026 17:33:09 -0400
Steven Rostedt <[email protected]> wrote:
> Function arguments at kretprobe
> -------------------------------
> diff --git a/kernel/trace/trace_btf.c b/kernel/trace/trace_btf.c
> index 00172f301f25..be88cc4d97dd 100644
> --- a/kernel/trace/trace_btf.c
> +++ b/kernel/trace/trace_btf.c
> @@ -120,3 +120,114 @@ const struct btf_member *btf_find_struct_member(struct
> btf *btf,
> return member;
> }
>
> +#define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
> +
> +static int find_member(const char *ptr, struct btf *btf,
> + const struct btf_type **type, int level)
We already have a similar function.
/*
* Find a member of data structure/union by name and return it.
* Return NULL if not found, or -EINVAL if parameter is invalid.
* If the member is an member of anonymous union/structure, the offset
* of that anonymous union/structure is stored into @anon_offset. Caller
* can calculate the correct offset from the root data structure by
* adding anon_offset to the member's offset.
*/
const struct btf_member *btf_find_struct_member(struct btf *btf,
const struct btf_type *type,
const char *member_name,
u32 *anon_offset)
And this seems useful for you,
> +{
> + const struct btf_member *member;
> + const struct btf_type *t = *type;
> + int i;
> +
> + /* Max of 3 depth of anonymous structures */
> + if (level > 3)
> + return -E2BIG;
> +
> + for_each_member(i, t, member) {
> + const char *tname = btf_name_by_offset(btf, member->name_off);
> +
> + if (strcmp(ptr, tname) == 0) {
> + int offset = __btf_member_bit_offset(t, member);
> + *type = btf_type_by_id(btf, member->type);
> + return BITS_ROUNDDOWN_BYTES(offset);
> + }
> +
> + /* Handle anonymous structures */
> + if (strlen(tname))
> + continue;
> +
> + *type = btf_type_by_id(btf, member->type);
> + if (btf_type_is_struct(*type)) {
> + int offset = find_member(ptr, btf, type, level + 1);
> +
> + if (offset < 0)
> + continue;
> +
> + return offset + BITS_ROUNDDOWN_BYTES(member->offset);
> + }
> + }
> +
> + return -ENOENT;
> +}
> +
> +/**
> + * btf_find_offset - Find an offset of a member for a structure
> + * @arg: A structure name followed by one or more members
> + * @offset_p: A pointer to where to store the offset
> + *
> + * Will parse @arg with the expected format of: struct.member[[.member]..]
> + * It is delimited by '.'. The first item must be a structure type.
> + * The next are its members. If the member is also of a structure type it
> + * another member may follow ".member".
> + *
> + * Note, @arg is modified but will be put back to what it was on return.
> + *
> + * Returns: 0 on success and -EINVAL if no '.' is present
> + * or -ENXIO if the structure or member is not found.
> + * Returns -EINVAL if BTF is not defined.
> + * On success, @offset_p will contain the offset of the member specified
> + * by @arg.
> + */
> +int btf_find_offset(char *arg, long *offset_p)
And this can share the inner loop of parse_btf_field().
(BTW, parse_btf_field() supports bitfield, but this does not.)
> +{
> + const struct btf_type *t;
> + struct btf *btf;
As Sashiko pointed, btf = NULL here.
But I think we would better to have DEFINE_FREE(btf_put).
> + long offset = 0;
> + char *ptr;
> + int ret;
> + s32 id;
> +
> + ptr = strchr(arg, '.');
> + if (!ptr)
> + return -EINVAL;
> +
> + *ptr = '\0';
> +
> + ret = -ENXIO;
> + id = bpf_find_btf_id(arg, BTF_KIND_STRUCT, &btf);
> + if (id < 0)
> + goto error;
> +
> + /* Get BTF_KIND_FUNC type */
> + t = btf_type_by_id(btf, id);
> +
> + /* May allow more than one member, as long as they are structures */
> + do {
> + ret = -ENXIO;
> + if (!t || !btf_type_is_struct(t))
> + goto error;
> +
> + *ptr++ = '.';
> + arg = ptr;
> + ptr = strchr(ptr, '.');
> + if (ptr)
> + *ptr = '\0';
> +
So, if you don't share the inner loop, you can just reuse
btf_field_struct_member() as below (this should be equivalent
to find_member()):
field = btf_find_struct_member(btf, t, arg, &anon_offs);
if (IS_ERR_OR_NULL(field)) {
ret = field ? PTR_ERR(field) : -ENOENT;
goto error;
}
offset += field->offset + anon_offs;
> +
> + } while (ptr);
> +
> + btf_put(btf);
> + *offset_p = offset;
> + return 0;
> +
> +error:
> + btf_put(btf);
> + if (ptr)
> + *ptr = '.';
> + return ret;
> +}
> diff --git a/kernel/trace/trace_btf.h b/kernel/trace/trace_btf.h
> index 4bc44bc261e6..7b0797a6050b 100644
> --- a/kernel/trace/trace_btf.h
> +++ b/kernel/trace/trace_btf.h
> @@ -9,3 +9,13 @@ const struct btf_member *btf_find_struct_member(struct btf
> *btf,
> const struct btf_type *type,
> const char *member_name,
> u32 *anon_offset);
> +
> +#ifdef CONFIG_PROBE_EVENTS_BTF_ARGS
> +/* Will modify arg, but will put it back before returning. */
> +int btf_find_offset(char *arg, long *offset);
> +#else
> +static inline int btf_find_offset(char *arg, long *offset)
> +{
> + return -EINVAL;
> +}
> +#endif
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index e0d3a0da26af..6fcede2de1a5 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1165,7 +1165,7 @@ parse_probe_arg(char *arg, const struct fetch_type
> *type,
>
> case '+': /* deref memory */
> case '-':
> - if (arg[1] == 'u') {
> + if (arg[1] == 'u' && isdigit(arg[2])) {
As Sashiko pointed, This can be broken if STRUCT name starts
with "u[0-9]". What about using "()" for this case?
e.g.
+u(user_unreg.disable_addr)(uarg)
> deref = FETCH_OP_UDEREF;
> arg[1] = arg[0];
> arg++;
> @@ -1178,7 +1178,22 @@ parse_probe_arg(char *arg, const struct fetch_type
> *type,
> return -EINVAL;
> }
> *tmp = '\0';
> - ret = kstrtol(arg, 0, &offset);
> + if (arg[0] != '-' && !isdigit(*arg)) {
> + int err = 0;
> + ret = btf_find_offset(arg, &offset);
> + switch (ret) {
> + case -ENODEV: err = TP_ERR_NOSUP_BTFARG; break;
> + case -E2BIG: err = TP_ERR_MEMBER_TOO_DEEP; break;
> + case -EINVAL: err = TP_ERR_BAD_STRUCT_FMT; break;
> + case -ENXIO: err = TP_ERR_BAD_BTF_TID; break;
This should have -ENOENT and default case for unknown error.
(There is TP_ERR_NO_BTF_FIELD already)
> + }
> + if (err)
> + __trace_probe_log_err(ctx->offset, err);
> + if (ret < 0)
> + return ret;
> + } else {
> + ret = kstrtol(arg, 0, &offset);
> + }
> if (ret) {
> trace_probe_log_err(ctx->offset, BAD_DEREF_OFFS);
> break;
> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 262d8707a3df..d649bb9f5b7c 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -563,7 +563,9 @@ extern int traceprobe_define_arg_fields(struct
> trace_event_call *event_call,
> C(NEED_STRING_TYPE, "$comm and immediate-string only accepts string
> type"),\
> C(TOO_MANY_ARGS, "Too many arguments are specified"), \
> C(TOO_MANY_EARGS, "Too many entry arguments specified"), \
> - C(EVENT_TOO_BIG, "Event too big (too many fields?)"),
> + C(EVENT_TOO_BIG, "Event too big (too many fields?)"), \
> + C(MEMBER_TOO_DEEP, "Too many indirections of anonymous
> structure"), \
> + C(BAD_STRUCT_FMT, "Unknown BTF structure"),
>
> #undef C
> #define C(a, b) TP_ERR_##a
> --
> 2.53.0
>
Thanks,
--
Masami Hiramatsu (Google) <[email protected]>
