Re: [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in kprobe_lookup_name()
From: Paul Clarke <hidden>
Date: 2017-04-21 13:52:20
Also in:
lkml
Sent too soon. The suggestions don't guarantee null termination. Refined, below. (Sorry for the noise.) On 04/21/2017 08:33 AM, Paul Clarke wrote:
On 04/21/2017 07:33 AM, Naveen N. Rao wrote:quoted
Convert usage of strchr()/strncpy()/strncat() to strnchr()/memcpy()/strlcat() for simpler and safer string manipulation.
quoted
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);
keep the null termination, and just adjust the starting point for strlcat: -- dot_name[modsym - name + 1] = '\0'; strlcat(&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?)quoted
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)
nevermind. :-)
quoted
} } 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)
nevermind. :-)
quoted
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)quoted
}
nevermind. :-) PC