[PATCH v2] powerpc/kprobes: refactor kprobe_lookup_name for safer string operations

[David Laight]

From: Naveen N. Rao [mailto:naveen.n.rao at 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.Laight at ACULAB.COM>
> Signed-off-by: Naveen N. Rao <naveen.n.rao at 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