All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	x86@kernel.org, Jinchao Wang <wangjinchao600@gmail.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Ian Rogers <irogers@google.com>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 2/2] tracing/probes: Fix BTF kflag check for anonymous struct member access
Date: Tue, 1 Sep 2026 07:37:51 +0900	[thread overview]
Message-ID: <20260901073751.aa53249adf57cdbae351a3a7@kernel.org> (raw)
In-Reply-To: <20260831131429.0c949991@gandalf.local.home>

On Mon, 31 Aug 2026 13:14:29 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue,  1 Sep 2026 00:06:16 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> > diff --git a/kernel/trace/trace_btf.h b/kernel/trace/trace_btf.h
> > index 4bc44bc261e6..4bd26bceae23 100644
> > --- a/kernel/trace/trace_btf.h
> > +++ b/kernel/trace/trace_btf.h
> > @@ -8,4 +8,5 @@ const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
> >  const struct btf_member *btf_find_struct_member(struct btf *btf,
> >  						const struct btf_type *type,
> >  						const char *member_name,
> > -						u32 *anon_offset);
> > +						u32 *anon_offset,
> > +						const struct btf_type **member_type);
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index c4163904ba74..ce2a7bb00d28 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -625,6 +625,7 @@ static int get_bitoffset_of_field(char **pfieldname, const struct btf_type **pty
> >  {
> >  	const struct btf_type *type = *ptype;
> >  	const struct btf_member *field;
> > +	const struct btf_type *mtype = NULL;
> 
> Why initialize mtype to NULL?
> 
> >  	struct btf *btf = ctx_btf(ctx);
> >  	char *fieldname = *pfieldname;
> >  	int bitoffs = 0;
> > @@ -640,7 +641,7 @@ static int get_bitoffset_of_field(char **pfieldname, const struct btf_type **pty
> >  
> >  		anon_offs = 0;
> >  		field = btf_find_struct_member(btf, type, fieldname,
> > -						&anon_offs);
> > +						&anon_offs, &mtype);
> 
> If mtype is not set here, then field would be either an error or NULL.
> 
> >  		if (IS_ERR(field)) {
> >  			trace_probe_log_err(ctx->offset, BAD_BTF_TID);
> >  			return PTR_ERR(field);
> 
> If field is an error or NULL, it exits out early.
> 
> > @@ -653,7 +654,7 @@ static int get_bitoffset_of_field(char **pfieldname, const struct btf_type **pty
> >  		bitoffs += anon_offs;
> >  
> >  		/* Accumulate the bit-offsets of the dot-connected fields */
> > -		if (btf_type_kflag(type)) {
> > +		if (btf_type_kflag(mtype)) {
> 
> mtype should be guaranteed as set here (maybe set to NULL, but set regardless)
> 
> Now, if btf_find_struct_member() did not set mtype, because this is in a
> loop, in a second iteration, mtype would be stale.

Ah, indeed!

> 
> So either set it to NULL at the top of the loop, or don't set it at all.
> 
> Or am I missing something?

OK, let me just drop NULL initialization because it is a kind of hardening.
(Since anon_offs is left uninitialized, mtype = NULL seems somewhat unbalanced.)

Thanks!

> 
> -- Steve
> 
> 
> 
> >  			bitoffs += BTF_MEMBER_BIT_OFFSET(field->offset);
> >  			ctx->last_bitsize = BTF_MEMBER_BITFIELD_SIZE(field->offset);
> >  		} else {
> > @@ -661,11 +662,11 @@ static int get_bitoffset_of_field(char **pfieldname, const struct btf_type **pty
> >  			ctx->last_bitsize = 0;
> >  		}
> >  
> > -			type = btf_type_skip_modifiers(btf, field->type, NULL);
> > -			if (!type) {
> > -				trace_probe_log_err(ctx->offset, BAD_BTF_TID);
> > -				return -EINVAL;
> > -			}
> > +		type = btf_type_skip_modifiers(btf, field->type, NULL);
> > +		if (!type) {
> > +			trace_probe_log_err(ctx->offset, BAD_BTF_TID);
> > +			return -EINVAL;
> > +		}
> >  
> >  		if (next)
> >  			ctx->offset += next - fieldname;
> 


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

  reply	other threads:[~2026-08-31 22:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-31 15:05 [PATCH 0/2] tracing/probes: Fix BTF structure member finder Masami Hiramatsu (Google)
2026-08-31 15:06 ` [PATCH 1/2] tracing/probes: Fix anon_stack check for unnamed bitfields in btf_find_struct_member Masami Hiramatsu (Google)
2026-08-31 17:01   ` Steven Rostedt
2026-08-31 22:11     ` Masami Hiramatsu
2026-08-31 18:11   ` sashiko-bot
2026-08-31 15:06 ` [PATCH 2/2] tracing/probes: Fix BTF kflag check for anonymous struct member access Masami Hiramatsu (Google)
2026-08-31 17:14   ` Steven Rostedt
2026-08-31 22:37     ` Masami Hiramatsu [this message]
2026-08-31 18:19   ` sashiko-bot

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=20260901073751.aa53249adf57cdbae351a3a7@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=irogers@google.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=wangjinchao600@gmail.com \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.