linux-kernel - Re: [PATCH v4 3/7] kprobes: validate the symbol name provided during probe registration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <dc950d45-4e98-9bd7-d435-4a6005507f94@us.ibm.com>
Date:   Fri, 21 Apr 2017 08:11:40 -0500
From:   Paul Clarke <pc@...ibm.com>
To:     "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Masami Hiramatsu <mhiramat@...nel.org>
Cc:     linuxppc-dev@...ts.ozlabs.org, Ingo Molnar <mingo@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/7] kprobes: validate the symbol name provided during
 probe registration

a nit or two, below...

On 04/21/2017 07:32 AM, Naveen N. Rao wrote:
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 6a128f3a7ed1..ff9b1ac72a38 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1383,6 +1383,34 @@ bool within_kprobe_blacklist(unsigned long addr)
>  }
> 
>  /*
> + * We mainly want to ensure that the provided string is of a reasonable length
> + * and is of the form [<mod_name>:]<sym_name>, so that this is safe to process
> + * further.
> + * We don't worry about invalid characters as those will just prevent
> + * matching existing kallsyms.
> + */
> +bool is_valid_kprobe_symbol_name(const char *name)
> +{
> +	size_t sym_len;
> +	const char *s;
> +
> +	s = strnchr(name, ':', MODULE_NAME_LEN + KSYM_NAME_LEN + 1);
> +	if (s) {
> +		sym_len = (size_t)(s - name);
> +		if (sym_len <= 0  || sym_len >= MODULE_NAME_LEN)

"sym_len <= 0" looks odd here, since sym_len is likely unsigned and would never be less than zero, anyway.

> +			return false;
> +		s++;
> +	} else
> +		s = name;
> +
> +	sym_len = strnlen(s, KSYM_NAME_LEN);
> +	if (sym_len <= 0 || sym_len >= KSYM_NAME_LEN)

here, too.

> +		return false;
> +
> +	return true;
> +}

PC