From: Paul Clarke <pc@us.ibm.com>
To: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
Michael Ellerman <mpe@ellerman.id.au>,
Masami Hiramatsu <mhiramat@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org, Ingo Molnar <mingo@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in kprobe_lookup_name()
Date: Fri, 21 Apr 2017 08:52:09 -0500 [thread overview]
Message-ID: <e3b3a444-fa98-5ed3-7dd7-a45d93db8b1f@us.ibm.com> (raw)
In-Reply-To: <5393535f-4469-d6b7-f7ce-256a5612999f@us.ibm.com>
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:
>> Convert usage of strchr()/strncpy()/strncat() to
>> strnchr()/memcpy()/strlcat() for simpler and safer string manipulation.
>> 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?)
>
>> 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. :-)
>
>> }
>> } 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. :-)
>> 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)
>
>> }
nevermind. :-)
PC
next prev parent reply other threads:[~2017-04-21 13:52 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-19 12:50 [PATCH v3 0/7] powerpc: a few kprobe fixes and refactoring Naveen N. Rao
2017-04-19 12:51 ` [PATCH v3 1/7] kprobes: convert kprobe_lookup_name() to a function Naveen N. Rao
2017-04-24 22:47 ` [v3,1/7] " Michael Ellerman
2017-04-19 12:51 ` [PATCH v3 2/7] powerpc: kprobes: fix handling of function offsets on ABIv2 Naveen N. Rao
2017-04-24 22:47 ` [v3,2/7] " Michael Ellerman
2017-04-19 12:51 ` [PATCH v3 3/7] kprobes: validate the symbol name length Naveen N. Rao
2017-04-19 14:37 ` Masami Hiramatsu
2017-04-19 16:38 ` Naveen N. Rao
2017-04-21 13:42 ` Masami Hiramatsu
2017-04-23 15:44 ` Naveen N. Rao
2017-04-25 3:18 ` Masami Hiramatsu
2017-04-20 6:08 ` Michael Ellerman
2017-04-20 7:19 ` Naveen N. Rao
2017-04-21 12:32 ` [PATCH v4 3/7] kprobes: validate the symbol name provided during probe registration Naveen N. Rao
2017-04-21 13:11 ` Paul Clarke
2017-04-21 13:25 ` Naveen N. Rao
2017-04-21 13:54 ` Masami Hiramatsu
2017-04-22 5:55 ` Michael Ellerman
2017-04-23 17:41 ` Naveen N. Rao
2017-04-21 12:33 ` [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in kprobe_lookup_name() Naveen N. Rao
2017-04-21 13:33 ` Paul Clarke
2017-04-21 13:36 ` Paul Clarke
2017-04-21 13:52 ` Paul Clarke [this message]
2017-04-23 17:09 ` Naveen N. Rao
2017-04-19 12:51 ` [PATCH v3 4/7] powerpc: kprobes: use " Naveen N. Rao
2017-04-21 15:06 ` David Laight
2017-04-19 12:51 ` [PATCH v3 5/7] powerpc: kprobes: factor out code to emulate instruction into a helper Naveen N. Rao
2017-04-19 14:40 ` Masami Hiramatsu
2017-04-24 22:47 ` [v3, " Michael Ellerman
2017-04-19 12:51 ` [PATCH v3 6/7] powerpc: kprobes: emulate instructions on kprobe handler re-entry Naveen N. Rao
2017-04-19 14:43 ` Masami Hiramatsu
2017-04-19 16:42 ` Naveen N. Rao
2017-04-20 6:11 ` Michael Ellerman
2017-04-21 13:48 ` Masami Hiramatsu
2017-04-24 22:47 ` [v3, " Michael Ellerman
2017-04-19 12:51 ` [PATCH v3 7/7] powerpc: kprobes: remove duplicate saving of msr Naveen N. Rao
2017-04-19 14:43 ` Masami Hiramatsu
2017-04-23 11:53 ` [v3,7/7] " Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e3b3a444-fa98-5ed3-7dd7-a45d93db8b1f@us.ibm.com \
--to=pc@us.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
--cc=mpe@ellerman.id.au \
--cc=naveen.n.rao@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).