Messages in this thread | | | From | Song Liu <> | Subject | Re: [PATCH v2] perf/core: use strndup_user() instead of buggy open-coded version | Date | Thu, 21 Feb 2019 07:12:10 +0000 |
| |
> On Feb 20, 2019, at 4:20 PM, Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Hi Jann, > > On Wed, 20 Feb 2019 17:54:43 +0100 > Jann Horn <jannh@google.com> wrote: > >> The first version of this method was missing the check for >> `ret == PATH_MAX`; then such a check was added, but it didn't call kfree() >> on error, so there was still a small memory leak in the error case. >> Fix it by using strndup_user() instead of open-coding it. >> > > This looks good to me. > > Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> > > BTW, for stable, this is good. For the long term, I think we should > fix strndup_user() to return -E2BUG when the user string is longer > than max. > > Thank you, > >> Fixes: 0eadcc7a7bc0 ("perf/core: Fix perf_uprobe_init()") >> Signed-off-by: Jann Horn <jannh@google.com>
Thanks for the fix!
Acked-by: Song Liu <songliubraving@fb.com>
>> --- >> v2: >> - be compatible with existing error codes (Masami Hiramatsu) >> >> kernel/trace/trace_event_perf.c | 16 +++++++--------- >> 1 file changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c >> index 76217bbef815..4629a6104474 100644 >> --- a/kernel/trace/trace_event_perf.c >> +++ b/kernel/trace/trace_event_perf.c >> @@ -299,15 +299,13 @@ int perf_uprobe_init(struct perf_event *p_event, >> >> if (!p_event->attr.uprobe_path) >> return -EINVAL; >> - path = kzalloc(PATH_MAX, GFP_KERNEL); >> - if (!path) >> - return -ENOMEM; >> - ret = strncpy_from_user( >> - path, u64_to_user_ptr(p_event->attr.uprobe_path), PATH_MAX); >> - if (ret == PATH_MAX) >> - return -E2BIG; >> - if (ret < 0) >> - goto out; >> + >> + path = strndup_user(u64_to_user_ptr(p_event->attr.uprobe_path), >> + PATH_MAX); >> + if (IS_ERR(path)) { >> + ret = PTR_ERR(path); >> + return (ret == -EINVAL) ? -E2BIG : ret; >> + } >> if (path[0] == '\0') { >> ret = -EINVAL; >> goto out; >> -- >> 2.21.0.rc0.258.g878e2cd30e-goog >> > > > -- > Masami Hiramatsu <mhiramat@kernel.org>
| |