Re: [PATCH v2 03/13] tracing/probes: Fix BTF bitfield parsing and out-of-bounds buffer write

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] tracing/probes: wrong bitfield lshift on big-endian in
  parse_btf_bitfield()
--

commit e5cd5bf4ce18aa0d644893f624a1f1a581f88aeb
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

tracing/probes: Fix BTF bitfield parsing and out-of-bounds buffer write

This commit updates BTF bitfield parsing in trace_probe.c to compute byte
and bit offsets consistently, select a matching unsigned fetch type, and
size FETCH_OP_MOD_BF shifts to the selected fetch type.

> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index a880a90642d7c..3a57fe6b67cce 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c

[ ... ]

> @@ -858,13 +876,22 @@ static const struct fetch_type *find_fetch_type_from_btf_type(
>  }
>  
>  static int parse_btf_bitfield(struct fetch_insn **pcode,
> +			      const struct fetch_type *t,
>  			      struct traceprobe_parse_context *ctx)
>  {
>  	struct fetch_insn *code = *pcode;
>  
> -	if ((ctx->last_bitsize % 8 == 0) && ctx->last_bitoffs == 0)
> +	if ((ctx->last_bitsize == 0 ||
> +	     ctx->last_bitsize == BYTES_TO_BITS(t->size)) &&
> +	    ctx->last_bitoffs == 0)
>  		return 0;
>  
> +	if (ctx->last_bitsize == 0 ||
> +	    BYTES_TO_BITS(t->size) < (ctx->last_bitsize + ctx->last_bitoffs)) {
> +		trace_probe_log_err(ctx->offset, BAD_BITFIELD);
> +		return -EINVAL;
> +	}
> +
>  	code++;
>  	if (code->op != FETCH_OP_NOP) {
>  		trace_probe_log_err(ctx->offset, TOO_MANY_OPS);
> @@ -873,9 +900,9 @@ static int parse_btf_bitfield(struct fetch_insn **pcode,
>  	*pcode = code;
>  
>  	code->op = FETCH_OP_MOD_BF;
> -	code->lshift = 64 - (ctx->last_bitsize + ctx->last_bitoffs);
> -	code->rshift = 64 - ctx->last_bitsize;
> -	code->basesize = 64 / 8;
> +	code->lshift = BYTES_TO_BITS(t->size) - (ctx->last_bitsize + ctx->last_bitoffs);
> +	code->rshift = BYTES_TO_BITS(t->size) - ctx->last_bitsize;
> +	code->basesize = t->size;
>  	return 0;
>  }

[Severity: Medium]
Does this calculate the wrong code->lshift on __BIG_ENDIAN_BITFIELD
architectures?

At runtime, FETCH_OP_ST_MEM copies t->size bytes starting at bitoffs / 8
into buf, and fetch_apply_bitfield() shifts that native-endian integer left
by code->lshift and right by code->rshift.  On big-endian systems, BTF bit
offsets are numbered from the most significant bit of the first byte, which
is loaded into the most significant byte of the integer in buf, so there
are ctx->last_bitoffs bits above the bitfield rather than
BYTES_TO_BITS(t->size) - (ctx->last_bitsize + ctx->last_bitoffs).  Using
the little-endian formula shifts out the bitfield bits and records
corrupted values in the trace buffer on big-endian architectures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179444679892.345731.5271591537517106679.stgit@devnote2?part=3