Re: [RFC PATCH 1/7] tracing/probe: Check maxactive error cases
On Wed, 13 Mar 2019 10:51:56 -0400 Steven Rostedt wrote: > On Wed, 13 Mar 2019 23:37:39 +0900 > Masami Hiramatsu wrote: > > > > > So now 'p1:..." will error out and not just be ignored? > > > > Yes, I think it is better to tell user "your command has a > > meaningless option, maybe you made a mistake" than ignore that. > > > > OK, just making sure. Hope nobody complains about it ;-) > > Are these OK to add for the next merge window, or do you want them in > now? I could probably get them ready for -rc1. Yes, I think [1/7] to [3/7] would be better to go to 5.1. Thank you, -- Masami Hiramatsu
Re: [RFC PATCH 1/7] tracing/probe: Check maxactive error cases
On Wed, 13 Mar 2019 23:37:39 +0900 Masami Hiramatsu wrote: > > So now 'p1:..." will error out and not just be ignored? > > Yes, I think it is better to tell user "your command has a > meaningless option, maybe you made a mistake" than ignore that. > OK, just making sure. Hope nobody complains about it ;-) Are these OK to add for the next merge window, or do you want them in now? I could probably get them ready for -rc1. -- Steve
Re: [RFC PATCH 1/7] tracing/probe: Check maxactive error cases
On Wed, 13 Mar 2019 09:20:51 -0400
Steven Rostedt wrote:
> On Wed, 13 Mar 2019 21:27:42 +0900
> Masami Hiramatsu wrote:
>
> > Check maxactive on kprobe error case, because maxactive
> > is only for kretprobe, not for kprobe. Also, maxactive
> > should not be 0, it should be at least 1.
> >
> > Signed-off-by: Masami Hiramatsu
> > ---
> > kernel/trace/trace_kprobe.c | 10 +++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index d5fb09ebba8b..d47e12596f12 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -624,7 +624,11 @@ static int trace_kprobe_create(int argc, const char
> > *argv[])
> > if (event)
> > event++;
> >
> > - if (is_return && isdigit(argv[0][1])) {
> > + if (isdigit(argv[0][1])) {
> > + if (!is_return) {
> > + pr_info("Maxactive is not for kprobe");
>
> So now 'p1:..." will error out and not just be ignored?
Yes, I think it is better to tell user "your command has a
meaningless option, maybe you made a mistake" than ignore that.
Thank you,
>
> -- Steve
>
> > + return -EINVAL;
> > + }
> > if (event)
> > len = event - [0][1] - 1;
> > else
> > @@ -634,8 +638,8 @@ static int trace_kprobe_create(int argc, const char
> > *argv[])
> > memcpy(buf, [0][1], len);
> > buf[len] = '\0';
> > ret = kstrtouint(buf, 0, );
> > - if (ret) {
> > - pr_info("Failed to parse maxactive.\n");
> > + if (ret || !maxactive) {
> > + pr_info("Invalid maxactive number\n");
> > return ret;
> > }
> > /* kretprobes instances are iterated over via a list. The
>
--
Masami Hiramatsu
Re: [RFC PATCH 1/7] tracing/probe: Check maxactive error cases
On Wed, 13 Mar 2019 21:27:42 +0900
Masami Hiramatsu wrote:
> Check maxactive on kprobe error case, because maxactive
> is only for kretprobe, not for kprobe. Also, maxactive
> should not be 0, it should be at least 1.
>
> Signed-off-by: Masami Hiramatsu
> ---
> kernel/trace/trace_kprobe.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index d5fb09ebba8b..d47e12596f12 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -624,7 +624,11 @@ static int trace_kprobe_create(int argc, const char
> *argv[])
> if (event)
> event++;
>
> - if (is_return && isdigit(argv[0][1])) {
> + if (isdigit(argv[0][1])) {
> + if (!is_return) {
> + pr_info("Maxactive is not for kprobe");
So now 'p1:..." will error out and not just be ignored?
-- Steve
> + return -EINVAL;
> + }
> if (event)
> len = event - [0][1] - 1;
> else
> @@ -634,8 +638,8 @@ static int trace_kprobe_create(int argc, const char
> *argv[])
> memcpy(buf, [0][1], len);
> buf[len] = '\0';
> ret = kstrtouint(buf, 0, );
> - if (ret) {
> - pr_info("Failed to parse maxactive.\n");
> + if (ret || !maxactive) {
> + pr_info("Invalid maxactive number\n");
> return ret;
> }
> /* kretprobes instances are iterated over via a list. The
[RFC PATCH 1/7] tracing/probe: Check maxactive error cases
Check maxactive on kprobe error case, because maxactive
is only for kretprobe, not for kprobe. Also, maxactive
should not be 0, it should be at least 1.
Signed-off-by: Masami Hiramatsu
---
kernel/trace/trace_kprobe.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index d5fb09ebba8b..d47e12596f12 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -624,7 +624,11 @@ static int trace_kprobe_create(int argc, const char
*argv[])
if (event)
event++;
- if (is_return && isdigit(argv[0][1])) {
+ if (isdigit(argv[0][1])) {
+ if (!is_return) {
+ pr_info("Maxactive is not for kprobe");
+ return -EINVAL;
+ }
if (event)
len = event - [0][1] - 1;
else
@@ -634,8 +638,8 @@ static int trace_kprobe_create(int argc, const char *argv[])
memcpy(buf, [0][1], len);
buf[len] = '\0';
ret = kstrtouint(buf, 0, );
- if (ret) {
- pr_info("Failed to parse maxactive.\n");
+ if (ret || !maxactive) {
+ pr_info("Invalid maxactive number\n");
return ret;
}
/* kretprobes instances are iterated over via a list. The
