* [PATCH] ftrace: Replace all non-returning strlcpy with strscpy
@ 2023-05-17 14:53 Azeem Shaikh
2023-05-17 19:14 ` Kees Cook
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Azeem Shaikh @ 2023-05-17 14:53 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: linux-hardening, Azeem Shaikh, Mark Rutland, linux-kernel,
linux-trace-kernel
strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().
No return values were used, so direct replacement is safe.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
[2] https://github.com/KSPP/linux/issues/89
Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
---
kernel/trace/ftrace.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 764668467155..6a77edb51f18 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5743,7 +5743,7 @@ bool ftrace_filter_param __initdata;
static int __init set_ftrace_notrace(char *str)
{
ftrace_filter_param = true;
- strlcpy(ftrace_notrace_buf, str, FTRACE_FILTER_SIZE);
+ strscpy(ftrace_notrace_buf, str, FTRACE_FILTER_SIZE);
return 1;
}
__setup("ftrace_notrace=", set_ftrace_notrace);
@@ -5751,7 +5751,7 @@ __setup("ftrace_notrace=", set_ftrace_notrace);
static int __init set_ftrace_filter(char *str)
{
ftrace_filter_param = true;
- strlcpy(ftrace_filter_buf, str, FTRACE_FILTER_SIZE);
+ strscpy(ftrace_filter_buf, str, FTRACE_FILTER_SIZE);
return 1;
}
__setup("ftrace_filter=", set_ftrace_filter);
@@ -5763,14 +5763,14 @@ static int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
static int __init set_graph_function(char *str)
{
- strlcpy(ftrace_graph_buf, str, FTRACE_FILTER_SIZE);
+ strscpy(ftrace_graph_buf, str, FTRACE_FILTER_SIZE);
return 1;
}
__setup("ftrace_graph_filter=", set_graph_function);
static int __init set_graph_notrace_function(char *str)
{
- strlcpy(ftrace_graph_notrace_buf, str, FTRACE_FILTER_SIZE);
+ strscpy(ftrace_graph_notrace_buf, str, FTRACE_FILTER_SIZE);
return 1;
}
__setup("ftrace_graph_notrace=", set_graph_notrace_function);
@@ -6569,8 +6569,8 @@ static int ftrace_get_trampoline_kallsym(unsigned int symnum,
continue;
*value = op->trampoline;
*type = 't';
- strlcpy(name, FTRACE_TRAMPOLINE_SYM, KSYM_NAME_LEN);
- strlcpy(module_name, FTRACE_TRAMPOLINE_MOD, MODULE_NAME_LEN);
+ strscpy(name, FTRACE_TRAMPOLINE_SYM, KSYM_NAME_LEN);
+ strscpy(module_name, FTRACE_TRAMPOLINE_MOD, MODULE_NAME_LEN);
*exported = 0;
return 0;
}
@@ -6933,7 +6933,7 @@ ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
if (off)
*off = addr - found_func->ip;
if (sym)
- strlcpy(sym, found_func->name, KSYM_NAME_LEN);
+ strscpy(sym, found_func->name, KSYM_NAME_LEN);
return found_func->name;
}
@@ -6987,8 +6987,8 @@ int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *value,
*value = mod_func->ip;
*type = 'T';
- strlcpy(name, mod_func->name, KSYM_NAME_LEN);
- strlcpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
+ strscpy(name, mod_func->name, KSYM_NAME_LEN);
+ strscpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
*exported = 1;
preempt_enable();
return 0;
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ftrace: Replace all non-returning strlcpy with strscpy
2023-05-17 14:53 [PATCH] ftrace: Replace all non-returning strlcpy with strscpy Azeem Shaikh
@ 2023-05-17 19:14 ` Kees Cook
2023-05-17 22:28 ` Masami Hiramatsu
2023-05-26 17:45 ` Kees Cook
2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2023-05-17 19:14 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Steven Rostedt, Masami Hiramatsu, linux-hardening, Mark Rutland,
linux-kernel, linux-trace-kernel
On Wed, May 17, 2023 at 02:53:23PM +0000, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
> No return values were used, so direct replacement is safe.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ftrace: Replace all non-returning strlcpy with strscpy
2023-05-17 14:53 [PATCH] ftrace: Replace all non-returning strlcpy with strscpy Azeem Shaikh
2023-05-17 19:14 ` Kees Cook
@ 2023-05-17 22:28 ` Masami Hiramatsu
2023-05-26 17:45 ` Kees Cook
2 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2023-05-17 22:28 UTC (permalink / raw)
To: Azeem Shaikh
Cc: Steven Rostedt, linux-hardening, Mark Rutland, linux-kernel,
linux-trace-kernel
On Wed, 17 May 2023 14:53:23 +0000
Azeem Shaikh <azeemshaikh38@gmail.com> wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
> No return values were used, so direct replacement is safe.
>
> [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
> [2] https://github.com/KSPP/linux/issues/89
>
> Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> ---
> kernel/trace/ftrace.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 764668467155..6a77edb51f18 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5743,7 +5743,7 @@ bool ftrace_filter_param __initdata;
> static int __init set_ftrace_notrace(char *str)
> {
> ftrace_filter_param = true;
> - strlcpy(ftrace_notrace_buf, str, FTRACE_FILTER_SIZE);
> + strscpy(ftrace_notrace_buf, str, FTRACE_FILTER_SIZE);
> return 1;
> }
> __setup("ftrace_notrace=", set_ftrace_notrace);
> @@ -5751,7 +5751,7 @@ __setup("ftrace_notrace=", set_ftrace_notrace);
> static int __init set_ftrace_filter(char *str)
> {
> ftrace_filter_param = true;
> - strlcpy(ftrace_filter_buf, str, FTRACE_FILTER_SIZE);
> + strscpy(ftrace_filter_buf, str, FTRACE_FILTER_SIZE);
> return 1;
> }
> __setup("ftrace_filter=", set_ftrace_filter);
> @@ -5763,14 +5763,14 @@ static int ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer);
>
> static int __init set_graph_function(char *str)
> {
> - strlcpy(ftrace_graph_buf, str, FTRACE_FILTER_SIZE);
> + strscpy(ftrace_graph_buf, str, FTRACE_FILTER_SIZE);
> return 1;
> }
> __setup("ftrace_graph_filter=", set_graph_function);
>
> static int __init set_graph_notrace_function(char *str)
> {
> - strlcpy(ftrace_graph_notrace_buf, str, FTRACE_FILTER_SIZE);
> + strscpy(ftrace_graph_notrace_buf, str, FTRACE_FILTER_SIZE);
> return 1;
> }
> __setup("ftrace_graph_notrace=", set_graph_notrace_function);
> @@ -6569,8 +6569,8 @@ static int ftrace_get_trampoline_kallsym(unsigned int symnum,
> continue;
> *value = op->trampoline;
> *type = 't';
> - strlcpy(name, FTRACE_TRAMPOLINE_SYM, KSYM_NAME_LEN);
> - strlcpy(module_name, FTRACE_TRAMPOLINE_MOD, MODULE_NAME_LEN);
> + strscpy(name, FTRACE_TRAMPOLINE_SYM, KSYM_NAME_LEN);
> + strscpy(module_name, FTRACE_TRAMPOLINE_MOD, MODULE_NAME_LEN);
> *exported = 0;
> return 0;
> }
> @@ -6933,7 +6933,7 @@ ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
> if (off)
> *off = addr - found_func->ip;
> if (sym)
> - strlcpy(sym, found_func->name, KSYM_NAME_LEN);
> + strscpy(sym, found_func->name, KSYM_NAME_LEN);
>
> return found_func->name;
> }
> @@ -6987,8 +6987,8 @@ int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *value,
>
> *value = mod_func->ip;
> *type = 'T';
> - strlcpy(name, mod_func->name, KSYM_NAME_LEN);
> - strlcpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
> + strscpy(name, mod_func->name, KSYM_NAME_LEN);
> + strscpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
> *exported = 1;
> preempt_enable();
> return 0;
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ftrace: Replace all non-returning strlcpy with strscpy
2023-05-17 14:53 [PATCH] ftrace: Replace all non-returning strlcpy with strscpy Azeem Shaikh
2023-05-17 19:14 ` Kees Cook
2023-05-17 22:28 ` Masami Hiramatsu
@ 2023-05-26 17:45 ` Kees Cook
2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2023-05-26 17:45 UTC (permalink / raw)
To: mhiramat, rostedt, azeemshaikh38
Cc: Kees Cook, linux-hardening, linux-trace-kernel, linux-kernel,
mark.rutland
On Wed, 17 May 2023 14:53:23 +0000, Azeem Shaikh wrote:
> strlcpy() reads the entire source buffer first.
> This read may exceed the destination size limit.
> This is both inefficient and can lead to linear read
> overflows if a source string is not NUL-terminated [1].
> In an effort to remove strlcpy() completely [2], replace
> strlcpy() here with strscpy().
> No return values were used, so direct replacement is safe.
>
> [...]
Applied to for-next/hardening, thanks!
[1/1] ftrace: Replace all non-returning strlcpy with strscpy
https://git.kernel.org/kees/c/bd35ef4f612f
--
Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-26 17:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-17 14:53 [PATCH] ftrace: Replace all non-returning strlcpy with strscpy Azeem Shaikh
2023-05-17 19:14 ` Kees Cook
2023-05-17 22:28 ` Masami Hiramatsu
2023-05-26 17:45 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).