> On Feb 20, 2019, at 4:20 PM, Masami Hiramatsu <[email protected]> wrote:
>
> Hi Jann,
>
> On Wed, 20 Feb 2019 17:54:43 +0100
> Jann Horn <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>
Thanks for the fix!
Acked-by: Song Liu <[email protected]>
>> ---
>> 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 <[email protected]>
