On Wed, 20 Mar 2024 12:44:23 +0900
Masami Hiramatsu (Google) [off-list ref] wrote:

quoted

quoted

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.

Initializing to zero is fine.

-- Steve