2009-12-11 08:01:25

by tip-bot for Wenji Huang

Subject: [PATCH]tracing: Fix inaccurate function definition in trace_events_filter.c

Use strcmp instead of strncmp to do full match.

Signed-off-by: Wenji Huang <[email protected]>
---
kernel/trace/trace_events_filter.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 50504cb..06dc376 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -254,7 +254,7 @@ static int filter_pred_none(struct filter_pred *pred, void *event,
/* Basic regex callbacks */
static int regex_match_full(char *str, struct regex *r, int len)
{
- if (strncmp(str, r->pattern, len) == 0)
+ if (strcmp(str, r->pattern) == 0)
return 1;
return 0;
}
--
1.5.6

2009-12-11 08:15:18

by Li Zefan

Subject: Re: [PATCH]tracing: Fix inaccurate function definition in trace_events_filter.c

Wenji Huang wrote:
> Use strcmp instead of strncmp to do full match.
>

It's intended to use strncmp instead of strcmp..

field(char, name, size) will be treated as a STATIC_STRING by
filter code. But this field isn't necessarily a string, it
can be an array of chars, which may not be ended with '\0'.
So in this case, we can't use strcmp.

So do you really see incorrect filter results?

I think STATIC_STRING and DYN_STRING should be fine, because
the "len" parameter is not the length of the string, but
the length of the array, which including the tailing '\0'.

For FILTER_PTR_STRING, the filter is not working correctly:

# echo 'func == vt' > events/bkl/lock_kernel/filter
# echo 1 > events/bkl/lock_kernel/enable
...
# cat trace
Xorg-1484 [000] 1973.392586: lock_kernel: ... func=vt_ioctl()
gpm-1402 [001] 1974.027740: lock_kernel: ... func=vt_ioctl()

Actually I already made 2 patches to fix this, and fix another
bug in regex_match_end(). I'll send them out soon.

> Signed-off-by: Wenji Huang <[email protected]>
> ---
> kernel/trace/trace_events_filter.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 50504cb..06dc376 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -254,7 +254,7 @@ static int filter_pred_none(struct filter_pred *pred, void *event,
> /* Basic regex callbacks */
> static int regex_match_full(char *str, struct regex *r, int len)
> {
> - if (strncmp(str, r->pattern, len) == 0)
> + if (strcmp(str, r->pattern) == 0)
> return 1;
> return 0;
> }

2009-12-11 14:39:24

by Steven Rostedt

Subject: Re: [PATCH]tracing: Fix inaccurate function definition in trace_events_filter.c

On Fri, 2009-12-11 at 16:15 +0800, Li Zefan wrote:
> Wenji Huang wrote:
> > Use strcmp instead of strncmp to do full match.
> >
>
> It's intended to use strncmp instead of strcmp..
>
> field(char, name, size) will be treated as a STATIC_STRING by
> filter code. But this field isn't necessarily a string, it
> can be an array of chars, which may not be ended with '\0'.
> So in this case, we can't use strcmp.
>
> So do you really see incorrect filter results?

Yeah, I agree that this should be strncmp. Have you seen errors?

-- Steve