Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1630165

Re: [PATCH v3 3/7] kprobes: validate the symbol name length

Show all headers | View raw

On Sun, 23 Apr 2017 15:44:32 +0000
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> >> >> +bool is_valid_kprobe_symbol_name(const char *name)
> >> > 
> >> > This just check the length of symbol_name buffer, and can contain
> >> > some invalid chars.
> >> 
> >> Yes, I kept the function name generic incase we would like to do more 
> >> validation in future, plus it's shorter than 
> >> is_valid_kprobe_symbol_name_len() ;-)
> > 
> > OK, if this is enough general, we'd better define this in
> > kernel/kallsyms.c or in kallsyms.h. Of course the function
> > should be called is_valid_symbol_name(). :-)
> 
> I actually think this should be done in kprobes itself. The primary 
> intent is to perform such validation right when we first obtain the 
> input from the user. In this case, however, kallsyms_lookup_name() is 
> also an exported symbol, so I do think some validation there would be 
> good to have as well.

IMHO, it is natural that kallsyms will know what is valid symbols.
Providing validation function by kprobes means kprobes also knows
that, and I concerns that may lead a double standard.

Thanks,

> >> >> +{
> >> >> +	size_t sym_len;
> >> >> +	char *s;
> >> >> +
> >> >> +	s = strchr(name, ':');
> >> 
> >> Hmm.. this should be strnchr(). I re-factored the code that moved the 
> >> strnlen() above this below. I'll fix this.
> >> 
> >> >> +	if (s) {
> >> >> +		sym_len = strnlen(s+1, KSYM_NAME_LEN);
> >> > 
> >> > If you use strnlen() here, you just need to ensure sym_len < KSYM_NAME_LEN.
> >> 
> >> Hmm.. not sure I follow. Are you saying the check for sym_len <= 0 is 
> >> not needed?
> > 
> > You can check sym_len != 0, but anyway, here we concern about
> > "longer" string (for performance reason), we can focus on
> > such case.
> > (BTW, could you also check the name != NULL at first?)
> > 
> > So, what I think it can be;
> > 
> > if (strnlen(s+1, KSYM_NAME_LEN) == KSYM_NAME_LEN ||
> >     (size_t)(s - name) >= MODULE_NAME_LEN)
> > 	return false;
> 
> Sure, thanks. I clearly need to refactor this code better!
> 
> - Naveen
> 
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v3 3/7] kprobes: validate the symbol name length "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-19 15:00 +0200
  Re: [PATCH v3 3/7] kprobes: validate the symbol name length Masami Hiramatsu <mhiramat@kernel.org> - 2017-04-19 17:40 +0200
    Re: [PATCH v3 3/7] kprobes: validate the symbol name length "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-19 18:40 +0200
      Re: [PATCH v3 3/7] kprobes: validate the symbol name length Masami Hiramatsu <mhiramat@kernel.org> - 2017-04-21 15:50 +0200
        Re: [PATCH v3 3/7] kprobes: validate the symbol name length "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-23 17:50 +0200
          Re: [PATCH v3 3/7] kprobes: validate the symbol name length Masami Hiramatsu <mhiramat@kernel.org> - 2017-04-25 05:20 +0200
  Re: [PATCH v3 3/7] kprobes: validate the symbol name length Michael Ellerman <mpe@ellerman.id.au> - 2017-04-20 08:10 +0200
    Re: [PATCH v3 3/7] kprobes: validate the symbol name length "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-20 09:30 +0200
  [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in kprobe_lookup_name() "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-21 14:40 +0200
    Re: [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in  kprobe_lookup_name() Paul Clarke <pc@us.ibm.com> - 2017-04-21 15:40 +0200
      Re: [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in  kprobe_lookup_name() Paul Clarke <pc@us.ibm.com> - 2017-04-21 15:40 +0200
      Re: [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in  kprobe_lookup_name() Paul Clarke <pc@us.ibm.com> - 2017-04-21 16:00 +0200
        Re: [PATCH v4 4/7] powerpc/kprobes: Use safer string functions in  kprobe_lookup_name() "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-23 19:20 +0200
  [PATCH v4 3/7] kprobes: validate the symbol name provided during probe registration "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-21 14:40 +0200
    Re: [PATCH v4 3/7] kprobes: validate the symbol name provided during  probe registration Paul Clarke <pc@us.ibm.com> - 2017-04-21 15:20 +0200
      Re: [PATCH v4 3/7] kprobes: validate the symbol name provided during  probe registration "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-21 15:30 +0200
    Re: [PATCH v4 3/7] kprobes: validate the symbol name provided  during probe registration Masami Hiramatsu <mhiramat@kernel.org> - 2017-04-21 16:00 +0200
    Re: [PATCH v4 3/7] kprobes: validate the symbol name provided during probe registration Michael Ellerman <mpe@ellerman.id.au> - 2017-04-22 08:00 +0200
      Re: [PATCH v4 3/7] kprobes: validate the symbol name provided during  probe registration "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-04-23 19:50 +0200

csiph-web