On Mon, 20 Nov 2023 05:46:13 +0530 Yuran Pereira <yuran.pere...@hotmail.com> wrote:
> The function simple_strtoul performs no error checking in scenarios > where the input value overflows the intended output variable. > This results in this function successfully returning, even when the > output does not match the input string (aka the function returns > successfully even when the result is wrong). > > Or as it was mentioned [1], "...simple_strtol(), simple_strtoll(), > simple_strtoul(), and simple_strtoull() functions explicitly ignore > overflows, which may lead to unexpected results in callers." > Hence, the use of those functions is discouraged. > > This patch replaces all uses of the simple_strtoul with the safer > alternatives kstrtoul and kstruint. > > Callers affected: > - add_rec_by_index > - set_graph_max_depth_function > > Side effects of this patch: > - Since `fgraph_max_depth` is an `unsigned int`, this patch uses > kstrtouint instead of kstrtoul to avoid any compiler warnings > that could originate from calling the latter. > - This patch ensures that the callers of kstrtou* return accordingly > when kstrtoul and kstruint fail for some reason. > In this case, both callers this patch is addressing return 0 on error. > > [1] > https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull > This looks good to me. Reviewed-by: Masami Hiramatsu (Google) <mhira...@kernel.org> Thank you! > Signed-off-by: Yuran Pereira <yuran.pere...@hotmail.com> > --- > kernel/trace/ftrace.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 8de8bec5f366..70217ee97322 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -4233,12 +4233,12 @@ static int > add_rec_by_index(struct ftrace_hash *hash, struct ftrace_glob *func_g, > int clear_filter) > { > - long index = simple_strtoul(func_g->search, NULL, 0); > + long index; > struct ftrace_page *pg; > struct dyn_ftrace *rec; > > /* The index starts at 1 */ > - if (--index < 0) > + if (kstrtoul(func_g->search, 0, &index) || --index < 0) > return 0; > > do_for_each_ftrace_rec(pg, rec) { > @@ -5810,9 +5810,8 @@ __setup("ftrace_graph_notrace=", > set_graph_notrace_function); > > static int __init set_graph_max_depth_function(char *str) > { > - if (!str) > + if (!str || kstrtouint(str, 0, &fgraph_max_depth)) > return 0; > - fgraph_max_depth = simple_strtoul(str, NULL, 0); > return 1; > } > __setup("ftrace_graph_max_depth=", set_graph_max_depth_function); > -- > 2.25.1 > -- Masami Hiramatsu (Google) <mhira...@kernel.org>
