Re: kernel/trace/trace_events_hist.c:6174 event_hist_trigger_parse() error: we previously assumed 'glob' could be null (see line 6166)

[Steven Rostedt]

On Wed, 26 Jan 2022 13:14:22 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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@xxxxxxxxx/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@xxxxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> 
> 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@xxxxxxxxxxxx