* [bug report] tracing: Allow whitespace to surround hist trigger filter
@ 2022-01-18 9:38 Dan Carpenter
2022-01-18 16:22 ` Zanussi, Tom
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-01-18 9:38 UTC (permalink / raw)
To: tom.zanussi; +Cc: Steven Rostedt, kernel-janitors
[ This is an older warning but renaming the function made it appear in
the new warnings list. I have searched my outbox and I don't think
I forwarded this one before. -dan ]
Hello Tom Zanussi,
The patch ec5ce0987541: "tracing: Allow whitespace to surround hist
trigger filter" from Jan 15, 2018, leads to the following Smatch
static checker warning:
kernel/trace/trace_events_hist.c:6199 event_hist_trigger_parse()
warn: 'p' can't be NULL.
kernel/trace/trace_events_hist.c
6149 static int event_hist_trigger_parse(struct event_command *cmd_ops,
6150 struct trace_event_file *file,
6151 char *glob, char *cmd, char *param)
6152 {
6153 unsigned int hist_trigger_bits = TRACING_MAP_BITS_DEFAULT;
6154 struct event_trigger_data *trigger_data;
6155 struct hist_trigger_attrs *attrs;
6156 struct event_trigger_ops *trigger_ops;
6157 struct hist_trigger_data *hist_data;
6158 struct synth_event *se;
6159 const char *se_name;
6160 bool remove = false;
6161 char *trigger, *p, *start;
6162 int ret = 0;
6163
6164 lockdep_assert_held(&event_mutex);
6165
6166 if (glob && strlen(glob)) {
6167 hist_err_clear();
6168 last_cmd_set(file, param);
6169 }
6170
6171 if (!param)
6172 return -EINVAL;
6173
6174 if (glob[0] == '!')
6175 remove = true;
6176
6177 /*
6178 * separate the trigger from the filter (k:v [if filter])
6179 * allowing for whitespace in the trigger
6180 */
6181 p = trigger = param;
6182 do {
6183 p = strstr(p, "if");
6184 if (!p)
6185 break;
6186 if (p == param)
6187 return -EINVAL;
6188 if (*(p - 1) != ' ' && *(p - 1) != '\t') {
6189 p++;
6190 continue;
^^^^^^^^^
These are the continue paths
6191 }
6192 if (p >= param + strlen(param) - (sizeof("if") - 1) - 1)
6193 return -EINVAL;
6194 if (*(p + sizeof("if") - 1) != ' ' && *(p + sizeof("if") - 1) != '\t') {
6195 p++;
6196 continue;
^^^^^^^^^
6197 }
6198 break;
--> 6199 } while (p);
Should this be } while(*p);?
6200
6201 if (!p)
6202 param = NULL;
6203 else {
6204 *(p - 1) = '\0';
6205 param = strstrip(p);
6206 trigger = strstrip(trigger);
6207 }
6208
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] tracing: Allow whitespace to surround hist trigger filter
2022-01-18 9:38 [bug report] tracing: Allow whitespace to surround hist trigger filter Dan Carpenter
@ 2022-01-18 16:22 ` Zanussi, Tom
0 siblings, 0 replies; 2+ messages in thread
From: Zanussi, Tom @ 2022-01-18 16:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Steven Rostedt, kernel-janitors
Hi Dan,
On 1/18/2022 3:38 AM, Dan Carpenter wrote:
> [ This is an older warning but renaming the function made it appear in
> the new warnings list. I have searched my outbox and I don't think
> I forwarded this one before. -dan ]
>
> Hello Tom Zanussi,
>
> The patch ec5ce0987541: "tracing: Allow whitespace to surround hist
> trigger filter" from Jan 15, 2018, leads to the following Smatch
> static checker warning:
>
> kernel/trace/trace_events_hist.c:6199 event_hist_trigger_parse()
> warn: 'p' can't be NULL.
>
> kernel/trace/trace_events_hist.c
> 6149 static int event_hist_trigger_parse(struct event_command *cmd_ops,
> 6150 struct trace_event_file *file,
> 6151 char *glob, char *cmd, char *param)
> 6152 {
> 6153 unsigned int hist_trigger_bits = TRACING_MAP_BITS_DEFAULT;
> 6154 struct event_trigger_data *trigger_data;
> 6155 struct hist_trigger_attrs *attrs;
> 6156 struct event_trigger_ops *trigger_ops;
> 6157 struct hist_trigger_data *hist_data;
> 6158 struct synth_event *se;
> 6159 const char *se_name;
> 6160 bool remove = false;
> 6161 char *trigger, *p, *start;
> 6162 int ret = 0;
> 6163
> 6164 lockdep_assert_held(&event_mutex);
> 6165
> 6166 if (glob && strlen(glob)) {
> 6167 hist_err_clear();
> 6168 last_cmd_set(file, param);
> 6169 }
> 6170
> 6171 if (!param)
> 6172 return -EINVAL;
> 6173
> 6174 if (glob[0] == '!')
> 6175 remove = true;
> 6176
> 6177 /*
> 6178 * separate the trigger from the filter (k:v [if filter])
> 6179 * allowing for whitespace in the trigger
> 6180 */
> 6181 p = trigger = param;
> 6182 do {
> 6183 p = strstr(p, "if");
> 6184 if (!p)
> 6185 break;
> 6186 if (p == param)
> 6187 return -EINVAL;
> 6188 if (*(p - 1) != ' ' && *(p - 1) != '\t') {
> 6189 p++;
> 6190 continue;
> ^^^^^^^^^
>
> These are the continue paths
>
> 6191 }
> 6192 if (p >= param + strlen(param) - (sizeof("if") - 1) - 1)
> 6193 return -EINVAL;
> 6194 if (*(p + sizeof("if") - 1) != ' ' && *(p + sizeof("if") - 1) != '\t') {
> 6195 p++;
> 6196 continue;
> ^^^^^^^^^
>
> 6197 }
> 6198 break;
> --> 6199 } while (p);
>
> Should this be } while(*p);?
No, it shouldn't be *p, it should be p, but the check at the top of the loop (if (!p) break) makes it redundant. I'll submit a patch to change it to while (1) to avoid the warning.
Thanks,
Tom
>
> 6200
> 6201 if (!p)
> 6202 param = NULL;
> 6203 else {
> 6204 *(p - 1) = '\0';
> 6205 param = strstrip(p);
> 6206 trigger = strstrip(trigger);
> 6207 }
> 6208
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-18 16:23 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-18 9:38 [bug report] tracing: Allow whitespace to surround hist trigger filter Dan Carpenter
2022-01-18 16:22 ` Zanussi, Tom
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.