[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230821120728.7b34266c@gandalf.local.home>
Date: Mon, 21 Aug 2023 12:07:28 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: Masami Hiramatsu <mhiramat@...nel.org>,
Beau Belgrave <beaub@...ux.microsoft.com>,
linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH] tracing/user_events: Fix an erroneous usage of
struct_size()
On Sun, 20 Aug 2023 17:02:42 +0200
Christophe JAILLET <christophe.jaillet@...adoo.fr> wrote:
> If struct_size() returns a value that does not fit in a 'int', the size
> passed to kzalloc() is wrong.
>
> Remove the intermediate 'size' variable and use struct_size() directly.
>
> Fixes: 7f5a08c79df3 ("user_events: Add minimal support for trace_event into ftrace")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
> I don't know if 'size' can get bigger than a int in the real world, but the
> change looks safe in any cases.
>
> On x86_64, looking at the .s files, the previous code had an extra:
> movslq %r13d, %r13
> which really looks wrong to me.
If size is bigger than int, then we have much bigger problems than this allocation.
That means count is over 2 billion, and the kzalloc() will fail regardless.
This is an unneeded change.
-- Steve
> ---
> kernel/trace/trace_events_user.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index 33cb6af31f39..67cc71a872b0 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -2153,7 +2153,7 @@ static int user_events_ref_add(struct user_event_file_info *info,
> {
> struct user_event_group *group = info->group;
> struct user_event_refs *refs, *new_refs;
> - int i, size, count = 0;
> + int i, count = 0;
>
> refs = rcu_dereference_protected(info->refs,
> lockdep_is_held(&group->reg_mutex));
> @@ -2166,10 +2166,8 @@ static int user_events_ref_add(struct user_event_file_info *info,
> return i;
> }
>
> - size = struct_size(refs, events, count + 1);
> -
> - new_refs = kzalloc(size, GFP_KERNEL_ACCOUNT);
> -
> + new_refs = kzalloc(struct_size(refs, events, count + 1),
> + GFP_KERNEL_ACCOUNT);
> if (!new_refs)
> return -ENOMEM;
>
Powered by blists - more mailing lists