Re: [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in kprobe_lookup_name()
From: Paul Clarke <hidden>
Date: 2017-04-21 13:33:35
Also in:
lkml
On 04/21/2017 07:33 AM, Naveen N. Rao wrote:
quoted hunk ↗ jump to hunk
Convert usage of strchr()/strncpy()/strncat() to strnchr()/memcpy()/strlcat() for simpler and safer string manipulation. Reported-by: David Laight <redacted> Signed-off-by: Naveen N. Rao <redacted> --- Changes: Additionally convert the strchr(). arch/powerpc/kernel/kprobes.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c index 97b5eed1f76d..c73fb6e3b43f 100644 --- a/arch/powerpc/kernel/kprobes.c +++ b/arch/powerpc/kernel/kprobes.c@@ -65,28 +65,27 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset) char dot_name[MODULE_NAME_LEN + 1 + KSYM_NAME_LEN]; const char *modsym; bool dot_appended = false; - if ((modsym = strchr(name, ':')) != NULL) { + if ((modsym = strnchr(name, ':', MODULE_NAME_LEN)) != NULL) { modsym++; if (*modsym != '\0' && *modsym != '.') { /* Convert to <module:.symbol> */ - strncpy(dot_name, name, modsym - name); + memcpy(dot_name, name, modsym - name); dot_name[modsym - name] = '.'; dot_name[modsym - name + 1] = '\0'; - strncat(dot_name, modsym, - sizeof(dot_name) - (modsym - name) - 2); + strlcat(dot_name, modsym, sizeof(dot_name));
Would it be more efficient here to replace this: -- dot_name[modsym - name + 1] = '\0'; strlcat(dot_name, modsym, sizeof(dot_name)); -- with this: strncpy(&dot_name[modsym - name + 1], modsym, KSYM_NAME_LEN); (So you aren't rescanning dot_name to find the end, when you already know the end position?)
dot_appended = true;
} else {
dot_name[0] = '\0';
- strncat(dot_name, name, sizeof(dot_name) - 1);
+ strlcat(dot_name, name, sizeof(dot_name));and here do: strncpy(dot_name, name, sizeof(dot_name)); (and remove the null termination immediately above)
}
} else if (name[0] != '.') {
dot_name[0] = '.';
dot_name[1] = '\0';
- strncat(dot_name, name, KSYM_NAME_LEN - 2);
+ strlcat(dot_name, name, sizeof(dot_name));and here do: strncpy(&dot_name[1], name, sizeof(dot_name)); (and remove the null termination immediately above)
dot_appended = true;
} else {
dot_name[0] = '\0';
- strncat(dot_name, name, KSYM_NAME_LEN - 1);
+ strlcat(dot_name, name, sizeof(dot_name));and here do: strncpy(dot_name, name, sizeof(dot_name)); (and remove the null termination immediately above)
}
PC