Linux Perf Users
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: sashiko-bot@kernel.org, linux-perf-users@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v13 12/12] tracing/wprobe: Support BTF struct offset resolution in set_wprobe trigger
Date: Sun, 30 Aug 2026 14:36:17 +0900	[thread overview]
Message-ID: <20260830143617.5edd4b9e94278a941ee6f28f@kernel.org> (raw)
In-Reply-To: <20260822095110.0772E1F000E9@smtp.kernel.org>

On Sat, 22 Aug 2026 09:51:09 +0000
sashiko-bot@kernel.org wrote:

> > @@ -990,6 +992,181 @@ static void wprobe_trigger_free(struct event_trigger_data *data)
> >  	}
> >  }
> >  
> > +#ifdef CONFIG_PROBE_EVENTS_BTF_ARGS
> > +
> > +static int get_offset_of_field(struct btf *btf, const struct btf_type *type, char *field_name)
> > +{
> > +	const struct btf_member *field;
> > +	int bitoffs = 0;
> > +	u32 anon_offs;
> > +	char *next;
> > +
> > +	do {
> > +		next = strchr(field_name, '.');
> > +		if (next)
> > +			*next++ = '\0';
> > +
> > +		field = btf_find_struct_member(btf, type, field_name, &anon_offs);
> > +		if (IS_ERR_OR_NULL(field))
> > +			return -ENOENT;
> > +
> > +		if (btf_type_kflag(type)) {
> 
> [Severity: High]
> Does this code check the kflag of the original parent type instead of the
> anonymous struct that actually contains the field?
> 
> btf_find_struct_member() can return a field from a nested anonymous struct,
> but if the parent struct and the anonymous struct have mismatched BTF kflags
> (for example, the parent has kflag=0 while the anonymous struct has kflag=1
> because it contains bitfields), using the parent's kflag here will cause
> the bitfield size encoded in the upper 8 bits of the offset to be
> misinterpreted as part of the byte offset.

Oops, that's right! btf_find_struct_member() skips anonymous struct and
get the field directly. Hmm, it seems the same issue in the trace_probe.c.
Let me make a fix patch.

> 
> This can add a massively inflated value to bitoffs and result in a wild
> pointer dereference when the wprobe triggers.
> 
> > +			/* Reject bitfield member access */
> > +			if (BTF_MEMBER_BITFIELD_SIZE(field->offset))
> > +				return -EINVAL;
> > +			bitoffs += anon_offs + BTF_MEMBER_BIT_OFFSET(field->offset);
> > +		} else {
> > +			bitoffs += anon_offs + field->offset;
> > +		}
> > +
> > +		field_name = next;
> > +		if (next) {
> > +			type = btf_type_skip_modifiers(btf, field->type, NULL);
> > +			if (!type)
> > +				return -ENOENT;
> > +		}
> > +	} while (next);
> > +	return bitoffs / BITS_PER_BYTE;
> > +}
> 
> [ ... ]
> 
> > +static int wprobe_trigger_typecast_parse(char **field_str_ptr,
> > +					 struct trace_event_file *file,
> > +					 struct wprobe_trigger_data *wprobe_data,
> > +					 const char *glob)
> > +{
> > +	struct btf *btf __free(btf_put) = NULL;
> > +	const struct btf_type *type;
> > +	char *assign_field;
> > +	char *event_field;
> > +	char *type_field;
> > +	char *type_name;
> > +	char *offs;
> > +	long val = 0;
> > +	int id;
> > +	int adjust;
> > +
> > +	type_name = *field_str_ptr + 1;
> > +	event_field = strchr(type_name, ')');
> > +	if (!event_field) {
> > +		wprobe_trigger_log_err(file, glob, type_name - glob, DEREF_OPEN_BRACE);
> 
> [Severity: Low]
> Will this log message place the caret out of bounds and hide the user's
> input?
> 
> The glob string passed into wprobe_trigger_log_err() seems to be truncated
> by an earlier strsep() call replacing the delimiter with a null terminator:
> 
> kernel/trace/trace_events_trigger.c:trigger_process_regex() {
>     ...
>     command = strsep(&next, ": \t");
>     ...
> }

Oops, strsep modifies the buffer. OK. let me fix it.

Thanks!


> 
> Since glob is truncated to just the command name, calculating the offset as
> type_name - glob can point far past the end of the printed text, making the
> syntax error logs illegible.
> 
> > +		return -EINVAL;
> > +	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/178739053919.1520941.17662338993878200834.stgit@devnote2?part=12


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

      reply	other threads:[~2026-08-30  5:36 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-22  9:22 [PATCH v13 00/12] tracing: wprobe: x86: Add wprobe for watchpoint Masami Hiramatsu (Google)
2026-08-22  9:22 ` [PATCH v13 01/12] kprobes: Protect kprobe_blacklist with RCU Masami Hiramatsu (Google)
2026-08-22  9:31   ` sashiko-bot
2026-08-22  9:22 ` [PATCH v13 02/12] x86/hw_breakpoints: Make DR7 updates NMI safe Masami Hiramatsu (Google)
2026-08-22  9:41   ` sashiko-bot
2026-08-30  5:35     ` Masami Hiramatsu
2026-08-22  9:22 ` [PATCH v13 03/12] x86/hw_breakpoints: Add arch_modify_local_hw_breakpoint_addr() API Masami Hiramatsu (Google)
2026-08-22  9:33   ` sashiko-bot
2026-08-22  9:23 ` [PATCH v13 04/12] HWBP: Add modify_local_hw_breakpoint_addr() API Masami Hiramatsu (Google)
2026-08-22  9:33   ` sashiko-bot
2026-08-22  9:24 ` [PATCH v13 05/12] tracing/wprobe: Add wprobe (watchpoint probe) trace event support Masami Hiramatsu (Google)
2026-08-22  9:49   ` sashiko-bot
2026-08-30  5:35     ` Masami Hiramatsu
2026-08-22  9:24 ` [PATCH v13 06/12] x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires Masami Hiramatsu (Google)
2026-08-22  9:31   ` sashiko-bot
2026-08-22  9:24 ` [PATCH v13 07/12] selftests: tracing: Add a basic testcase for wprobe Masami Hiramatsu (Google)
2026-08-22  9:36   ` sashiko-bot
2026-08-30  5:35     ` Masami Hiramatsu
2026-08-22  9:24 ` [PATCH v13 08/12] selftests: tracing: Add syntax " Masami Hiramatsu (Google)
2026-08-22  9:35   ` sashiko-bot
2026-08-30  5:35     ` Masami Hiramatsu
2026-08-22  9:24 ` [PATCH v13 09/12] tracing/wprobe: Add set_wprobe and clear_wprobe event triggers Masami Hiramatsu (Google)
2026-08-22  9:42   ` sashiko-bot
2026-08-30  5:35     ` Masami Hiramatsu
2026-08-22  9:25 ` [PATCH v13 10/12] selftests: ftrace: Add wprobe trigger testcase Masami Hiramatsu (Google)
2026-08-22  9:39   ` sashiko-bot
2026-08-30  5:36     ` Masami Hiramatsu
2026-08-22  9:25 ` [PATCH v13 11/12] tracing/wprobe: Support BTF typecast in fetchargs Masami Hiramatsu (Google)
2026-08-22  9:40   ` sashiko-bot
2026-08-22  9:25 ` [PATCH v13 12/12] tracing/wprobe: Support BTF struct offset resolution in set_wprobe trigger Masami Hiramatsu (Google)
2026-08-22  9:51   ` sashiko-bot
2026-08-30  5:36     ` Masami Hiramatsu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260830143617.5edd4b9e94278a941ee6f28f@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox