[bug report] tracing/kprobes: Add symbol counting check when module loads

Hello Masami Hiramatsu (Google),

Commit 9d8616034f16 ("tracing/kprobes: Add symbol counting check when
module loads") from Jul 5, 2024 (linux-next), leads to the following
Smatch static checker warning:

	kernel/trace/trace_kprobe.c:828 validate_probe_symbol()
	warn: missing error code here? 'try_module_get_by_name()' failed. 'ret' = '0'

kernel/trace/trace_kprobe.c
    815 static int validate_probe_symbol(char *symbol)
    816 {
    817         struct module *mod = NULL;
    818         char *modname = NULL, *p;
    819         int ret = 0;
    820 
    821         p = strchr(symbol, ':');
    822         if (p) {
    823                 modname = symbol;
    824                 symbol = p + 1;
    825                 *p = '\0';
    826                 mod = try_module_get_by_name(modname);
    827                 if (!mod)
--> 828                         goto out;

Is it intentional that "ret = 0;" here?

    829         }
    830 
    831         ret = validate_module_probe_symbol(modname, symbol);
    832 out:
    833         if (p)
    834                 *p = ':';
    835         if (mod)
    836                 module_put(mod);
    837         return ret;
    838 }

regards,
dan carpenter

On Thu, 18 Jul 2024 13:30:57 -0500
Dan Carpenter [off-list ref] wrote:

Hello Masami Hiramatsu (Google),

Commit 9d8616034f16 ("tracing/kprobes: Add symbol counting check when
module loads") from Jul 5, 2024 (linux-next), leads to the following
Smatch static checker warning:

	kernel/trace/trace_kprobe.c:828 validate_probe_symbol()
	warn: missing error code here? 'try_module_get_by_name()' failed. 'ret' = '0'

kernel/trace/trace_kprobe.c
    815 static int validate_probe_symbol(char *symbol)
    816 {
    817         struct module *mod = NULL;
    818         char *modname = NULL, *p;
    819         int ret = 0;
    820 
    821         p = strchr(symbol, ':');
    822         if (p) {
    823                 modname = symbol;
    824                 symbol = p + 1;
    825                 *p = '\0';
    826                 mod = try_module_get_by_name(modname);
    827                 if (!mod)
--> 828                         goto out;

Is it intentional that "ret = 0;" here?

Yes, hm, I dropped an important message here. If you see the commit
9d8616034f16, you can see there was;

/* Return 0 (defer) if the module does not exist yet. */

So it is intentional.

But thanks for reporting. It should be cleaned up.

Thank you,

    829         }
    830 
    831         ret = validate_module_probe_symbol(modname, symbol);
    832 out:
    833         if (p)
    834                 *p = ':';
    835         if (mod)
    836                 module_put(mod);
    837         return ret;
    838 }

regards,
dan carpenter

-- 
Masami Hiramatsu (Google) [off-list ref]