* kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166)
@ 2022-01-26 10:14 Dan Carpenter
2022-01-26 14:39 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2022-01-26 10:14 UTC (permalink / raw)
To: kbuild, Tom Zanussi; +Cc: lkp, kbuild-all, linux-kernel, Steven Rostedt
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0
commit: 9ec5a7d16899ed9062cc4c3dd3a13e1771411ab3 tracing: Change event_command func() to parse()
config: x86_64-randconfig-m001-20220124 (https://download.01.org/0day-ci/archive/20220125/202201250054.975KVd1O-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166)
vim +/glob +6174 kernel/trace/trace_events_hist.c
9ec5a7d16899ed Tom Zanussi 2022-01-10 6149 static int event_hist_trigger_parse(struct event_command *cmd_ops,
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6150 struct trace_event_file *file,
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6151 char *glob, char *cmd, char *param)
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6152 {
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6153 unsigned int hist_trigger_bits = TRACING_MAP_BITS_DEFAULT;
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6154 struct event_trigger_data *trigger_data;
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6155 struct hist_trigger_attrs *attrs;
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6156 struct event_trigger_ops *trigger_ops;
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6157 struct hist_trigger_data *hist_data;
4b147936fa5096 Tom Zanussi 2018-01-15 6158 struct synth_event *se;
4b147936fa5096 Tom Zanussi 2018-01-15 6159 const char *se_name;
30350d65ac5676 Tom Zanussi 2018-01-15 6160 bool remove = false;
c5eac6ee8bc5d3 Kalesh Singh 2021-10-25 6161 char *trigger, *p, *start;
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6162 int ret = 0;
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6163
0e2b81f7b52a1c Masami Hiramatsu 2018-11-05 6164 lockdep_assert_held(&event_mutex);
0e2b81f7b52a1c Masami Hiramatsu 2018-11-05 6165
f404da6e1d46ce Tom Zanussi 2018-01-15 @6166 if (glob && strlen(glob)) {
Check for NULL
f404da6e1d46ce Tom Zanussi 2018-01-15 6167 hist_err_clear();
a1a05bb40e229d Tom Zanussi 2019-03-31 6168 last_cmd_set(file, param);
f404da6e1d46ce Tom Zanussi 2018-01-15 6169 }
f404da6e1d46ce Tom Zanussi 2018-01-15 6170
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6171 if (!param)
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6172 return -EINVAL;
7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6173
30350d65ac5676 Tom Zanussi 2018-01-15 @6174 if (glob[0] == '!')
Unchecked dereference
30350d65ac5676 Tom Zanussi 2018-01-15 6175 remove = true;
30350d65ac5676 Tom Zanussi 2018-01-15 6176
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166)
2022-01-26 10:14 kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166) Dan Carpenter
@ 2022-01-26 14:39 ` Steven Rostedt
2022-01-26 14:50 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2022-01-26 14:39 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kbuild, Tom Zanussi, lkp, kbuild-all, linux-kernel
On Wed, 26 Jan 2022 13:14:22 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0
> commit: 9ec5a7d16899ed9062cc4c3dd3a13e1771411ab3 tracing: Change event_command func() to parse()
> config: x86_64-randconfig-m001-20220124 (https://download.01.org/0day-ci/archive/20220125/202201250054.975KVd1O-lkp@intel.com/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166)
>
> vim +/glob +6174 kernel/trace/trace_events_hist.c
>
> 9ec5a7d16899ed Tom Zanussi 2022-01-10 6149 static int event_hist_trigger_parse(struct event_command *cmd_ops,
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6150 struct trace_event_file *file,
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6151 char *glob, char *cmd, char *param)
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6152 {
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6153 unsigned int hist_trigger_bits = TRACING_MAP_BITS_DEFAULT;
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6154 struct event_trigger_data *trigger_data;
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6155 struct hist_trigger_attrs *attrs;
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6156 struct event_trigger_ops *trigger_ops;
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6157 struct hist_trigger_data *hist_data;
> 4b147936fa5096 Tom Zanussi 2018-01-15 6158 struct synth_event *se;
> 4b147936fa5096 Tom Zanussi 2018-01-15 6159 const char *se_name;
> 30350d65ac5676 Tom Zanussi 2018-01-15 6160 bool remove = false;
> c5eac6ee8bc5d3 Kalesh Singh 2021-10-25 6161 char *trigger, *p, *start;
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6162 int ret = 0;
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6163
> 0e2b81f7b52a1c Masami Hiramatsu 2018-11-05 6164 lockdep_assert_held(&event_mutex);
> 0e2b81f7b52a1c Masami Hiramatsu 2018-11-05 6165
> f404da6e1d46ce Tom Zanussi 2018-01-15 @6166 if (glob && strlen(glob)) {
I just reviewed the code, and it looks like the logic should keep glob from
ever being NULL.
I guess the solution could simply be to remove glob here, and perhaps add:
WARN_ON(!glob);
-- Steve
>
> Check for NULL
>
> f404da6e1d46ce Tom Zanussi 2018-01-15 6167 hist_err_clear();
> a1a05bb40e229d Tom Zanussi 2019-03-31 6168 last_cmd_set(file, param);
> f404da6e1d46ce Tom Zanussi 2018-01-15 6169 }
> f404da6e1d46ce Tom Zanussi 2018-01-15 6170
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6171 if (!param)
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6172 return -EINVAL;
> 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6173
> 30350d65ac5676 Tom Zanussi 2018-01-15 @6174 if (glob[0] == '!')
>
> Unchecked dereference
>
> 30350d65ac5676 Tom Zanussi 2018-01-15 6175 remove = true;
> 30350d65ac5676 Tom Zanussi 2018-01-15 6176
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166)
2022-01-26 14:39 ` Steven Rostedt
@ 2022-01-26 14:50 ` Dan Carpenter
2022-01-26 15:27 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2022-01-26 14:50 UTC (permalink / raw)
To: Steven Rostedt; +Cc: kbuild, Tom Zanussi, lkp, kbuild-all, linux-kernel
On Wed, Jan 26, 2022 at 09:39:18AM -0500, Steven Rostedt wrote:
> > 9ec5a7d16899ed Tom Zanussi 2022-01-10 6149 static int event_hist_trigger_parse(struct event_command *cmd_ops,
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6150 struct trace_event_file *file,
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6151 char *glob, char *cmd, char *param)
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6152 {
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6153 unsigned int hist_trigger_bits = TRACING_MAP_BITS_DEFAULT;
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6154 struct event_trigger_data *trigger_data;
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6155 struct hist_trigger_attrs *attrs;
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6156 struct event_trigger_ops *trigger_ops;
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6157 struct hist_trigger_data *hist_data;
> > 4b147936fa5096 Tom Zanussi 2018-01-15 6158 struct synth_event *se;
> > 4b147936fa5096 Tom Zanussi 2018-01-15 6159 const char *se_name;
> > 30350d65ac5676 Tom Zanussi 2018-01-15 6160 bool remove = false;
> > c5eac6ee8bc5d3 Kalesh Singh 2021-10-25 6161 char *trigger, *p, *start;
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6162 int ret = 0;
> > 7ef224d1d0e3a1 Tom Zanussi 2016-03-03 6163
> > 0e2b81f7b52a1c Masami Hiramatsu 2018-11-05 6164 lockdep_assert_held(&event_mutex);
> > 0e2b81f7b52a1c Masami Hiramatsu 2018-11-05 6165
> > f404da6e1d46ce Tom Zanussi 2018-01-15 @6166 if (glob && strlen(glob)) {
>
> I just reviewed the code, and it looks like the logic should keep glob from
> ever being NULL.
Smatch can also figure that out, and does not warn, if you have the
cross function DB. Unfortunately, that's not feasible for the zero day
bot because it takes too long to build the DB.
I was puzzled why this warning showed up now when the code is from 2018.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166)
2022-01-26 14:50 ` Dan Carpenter
@ 2022-01-26 15:27 ` Steven Rostedt
0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2022-01-26 15:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kbuild, Tom Zanussi, lkp, kbuild-all, linux-kernel
On Wed, 26 Jan 2022 17:50:20 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > I just reviewed the code, and it looks like the logic should keep glob from
> > ever being NULL.
>
> Smatch can also figure that out, and does not warn, if you have the
> cross function DB. Unfortunately, that's not feasible for the zero day
> bot because it takes too long to build the DB.
>
> I was puzzled why this warning showed up now when the code is from 2018.
Well it is called from trigger_process_regex() that gets buf passed in, and
that gets called by trace_boot_init_histograms(),
trace_boot_init_one_event() and event_trigger_regex_write(). All of which
pass in something for that glob parameter.
But I can see it being difficult to catch all that.
I have no problem adding the WARN_ON(), just to make it cleaner.
-- Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-26 15:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-26 10:14 kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166) Dan Carpenter
2022-01-26 14:39 ` Steven Rostedt
2022-01-26 14:50 ` Dan Carpenter
2022-01-26 15:27 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox