lkml.org 
[lkml]   [2017]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in kprobe_lookup_name()
    From
    Date
    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.
    >
    > Reported-by: David Laight <David.Laight@ACULAB.COM>
    > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
    > ---
    > Changes: Additionally convert the strchr().
    >
    >
    > arch/powerpc/kernel/kprobes.c | 13 ++++++-------
    > 1 file changed, 6 insertions(+), 7 deletions(-)
    >
    > 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);

    (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)

    > }
    > } 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)

    > 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)

    > }

    PC

    \
     
     \ /
      Last update: 2017-04-21 15:33    [W:7.858 / U:0.348 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site