From: Naveen N. Rao [mailto:naveen.n....@linux.vnet.ibm.com]
> Sent: 04 May 2017 11:25
> Use safer string manipulation functions when dealing with a
> user-provided string in kprobe_lookup_name().
>
> Reported-by: David Laight <david.lai...@aculab.com>
> Signed-off-by: Naveen N. Rao <naveen.n....@linux.vnet.ibm.com>
> ---
> Changed to ignore return value of 0 from strscpy(), as suggested by
> Masami.
let's see what this code looks like;
> char dot_name[MODULE_NAME_LEN + 1 + KSYM_NAME_LEN];
> bool dot_appended = false;
> + const char *c;
> + ssize_t ret = 0;
> + int len = 0;
> +
> + if ((c = strnchr(name, MODULE_NAME_LEN, ':')) != NULL) {
I don't like unnecessary assignments in conditionals.
> + c++;
> + len = c - name;
> + memcpy(dot_name, name, len);
> + } else
> + c = name;
> +
> + if (*c != '\0' && *c != '.') {
> + dot_name[len++] = '.';
> dot_appended = true;
If you don't append a dot, then you can always just lookup
the original string.
> }
> + ret = strscpy(dot_name + len, c, KSYM_NAME_LEN);
> + if (ret > 0)
> + addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name);
I'm not sure you need 'ret' here at all.
> + /* Fallback to the original non-dot symbol lookup */
> + if (!addr && dot_appended)
> addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
We can bikeshed this function to death:
/* The function name must start with a '.'.
* If it doesn't then we insert one. */
c = strnchr(name, MODULE_NAME_LEN, ':');
if (c && c[1] && c[1] != '.') {
/* Insert a '.' after the ':' */
c++;
len = c - name;
memcpy(dot_name, name, len);
} else {
if (name[0] == '.')
goto check_name:
/* Insert a '.' before name */
c = name;
len = 0;
}
dot_name[len++] = '.';
if (strscpy(dot_name + len, c, KSYM_NAME_LEN) > 0) {
addr = (kprobe_opcode_t *)kallsyms_lookup_name(dot_name);
if (addr)
return addr;
}
/* Symbol with extra '.' not found, fallback to original name */
check_name:
return (kprobe_opcode_t *)kallsyms_lookup_name(name);
David
