linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 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;
as well as URLs for NNTP newsgroup(s).