[bug report] fprobe: Skip exit_handler if entry_handler returns !0

[bug report] fprobe: Skip exit_handler if entry_handler returns !0

[Dan Carpenter]

Hello Masami Hiramatsu (Google),

The patch 39d954200bf6: "fprobe: Skip exit_handler if entry_handler
returns !0" from Feb 2, 2023, leads to the following Smatch static
checker warning:

	kernel/trace/fprobe.c:59 fprobe_handler()
	error: uninitialized symbol 'ret'.

kernel/trace/fprobe.c
    49                 fpr->entry_ip = ip;
    50                 if (fp->entry_data_size)
    51                         entry_data = fpr->data;
    52         }
    53 
    54         if (fp->entry_handler)
    55                 ret = fp->entry_handler(fp, ip, ftrace_get_regs(fregs), entry_data);

ret is only initialized if there is an ->entry_handler

    56 
    57         /* If entry_handler returns !0, nmissed is not counted. */
    58         if (rh) {

rh is only true if there is an ->exit_handler.  Presumably if you have
and ->exit_handler that means you also have a ->entry_handler but Smatch
is not smart enough to figure it out.

--> 59                 if (ret)
                           ^^^
Warning here.

    60                         rethook_recycle(rh);
    61                 else
    62                         rethook_hook(rh, ftrace_get_regs(fregs), true);
    63         }
    64 out:
    65         ftrace_test_recursion_unlock(bit);
    66 }

regards,
dan carpenter

Re: [bug report] fprobe: Skip exit_handler if entry_handler returns !0

[Masami Hiramatsu]

Hi Dan,

Thanks for reporting!

On Tue, 4 Apr 2023 10:37:48 +0300
Dan Carpenter <error27@gmail.com> wrote:

> Hello Masami Hiramatsu (Google),
> 
> The patch 39d954200bf6: "fprobe: Skip exit_handler if entry_handler
> returns !0" from Feb 2, 2023, leads to the following Smatch static
> checker warning:
> 
> 	kernel/trace/fprobe.c:59 fprobe_handler()
> 	error: uninitialized symbol 'ret'.
> 
> kernel/trace/fprobe.c
>     49                 fpr->entry_ip = ip;
>     50                 if (fp->entry_data_size)
>     51                         entry_data = fpr->data;
>     52         }
>     53 
>     54         if (fp->entry_handler)
>     55                 ret = fp->entry_handler(fp, ip, ftrace_get_regs(fregs), entry_data);
> 
> ret is only initialized if there is an ->entry_handler
> 
>     56 
>     57         /* If entry_handler returns !0, nmissed is not counted. */
>     58         if (rh) {
> 
> rh is only true if there is an ->exit_handler.  Presumably if you have
> and ->exit_handler that means you also have a ->entry_handler but Smatch
> is not smart enough to figure it out.

Ah, OK. Let me fix to initialize 'ret'.

> 
> --> 59                 if (ret)
>                            ^^^
> Warning here.

Thank you!

> 
>     60                         rethook_recycle(rh);
>     61                 else
>     62                         rethook_hook(rh, ftrace_get_regs(fregs), true);
>     63         }
>     64 out:
>     65         ftrace_test_recursion_unlock(bit);
>     66 }
> 
> regards,
> dan carpenter


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