Re: [bug report] tracing/probes: Support $argN in return probe (kprobe and fprobe)

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

On Tue, 19 Mar 2024 10:10:00 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 19 Mar 2024 10:19:09 +0300
> Dan Carpenter <dan.carpenter@linaro.org> wrote:
> 
> > Hello Masami Hiramatsu (Google),
> > 
> > Commit 25f00e40ce79 ("tracing/probes: Support $argN in return probe
> > (kprobe and fprobe)") from Mar 4, 2024 (linux-next), leads to the
> > following Smatch static checker warning:
> > 
> > 	kernel/trace/trace_probe.c:856 store_trace_entry_data()
> > 	error: uninitialized symbol 'val'.
> > 
> > kernel/trace/trace_probe.c
> >     846                 return;
> >     847 
> >     848         for (i = 0; i < earg->size; i++) {
> >     849                 struct fetch_insn *code = &earg->code[i];
> >     850 
> >     851                 switch (code->op) {
> >     852                 case FETCH_OP_ARG:
> >     853                         val = regs_get_kernel_argument(regs, code->param);
> >     854                         break;
> >     855                 case FETCH_OP_ST_EDATA:
> > --> 856                         *(unsigned long *)((unsigned long)edata + code->offset) = val;  
> > 
> > Probably the earg->code[i] always has FETCH_OP_ARG before
> > FETCH_OP_ST_EDATA but Smatch isn't smart enough to figure that out...
> 
> Looks that way:
> 
> 		case FETCH_OP_END:
> 			earg->code[i].op = FETCH_OP_ARG;
> 			earg->code[i].param = argnum;
> 			earg->code[i + 1].op = FETCH_OP_ST_EDATA;
> 			earg->code[i + 1].offset = offset;
> 			return offset;
> 
> But probably should still initialize val to zero or have a WARN_ON() if
> that doesn't happen.

OK, let's val = 0 in the store_trace_entry_data(), but WARN_ON() in this loop
is a bit strange. I think we should have a verifiler.

Thank you,

> 
> -- Steve
> 
> 
> > 
> >     857                         break;
> >     858                 case FETCH_OP_END:
> >     859                         goto end;
> >     860                 default:
> >     861                         break;
> >     862                 }
> >     863         }
> >     864 end:
> >     865         return;
> >     866 }
> > 
> > regards,
> > dan carpenter
> 


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