| Message ID | 20170425161822.18764-1-naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
From: Naveen N. Rao > Sent: 25 April 2017 17:18 > 1. Fail early for invalid/zero length symbols. > 2. Detect names of the form <mod:name> and skip checking for kernel > symbols in that case. > > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > Masami, Michael, > I have added two very simple checks here, which I felt is good to have, > rather than the elaborate checks in the previous version. Given the > change in module code to use strnchr(), the checks are now safe and > further tests are not probably not that useful. ... > + if (strnchr(name, MODULE_NAME_LEN, ':')) > + return module_kallsyms_lookup_name(name); Should that be MODULE_NAME_LEN - 1 ? David
Excerpts from David Laight's message of April 25, 2017 22:06: > From: Naveen N. Rao >> Sent: 25 April 2017 17:18 >> 1. Fail early for invalid/zero length symbols. >> 2. Detect names of the form <mod:name> and skip checking for kernel >> symbols in that case. >> >> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> >> --- >> Masami, Michael, >> I have added two very simple checks here, which I felt is good to have, >> rather than the elaborate checks in the previous version. Given the >> change in module code to use strnchr(), the checks are now safe and >> further tests are not probably not that useful. > ... >> + if (strnchr(name, MODULE_NAME_LEN, ':')) >> + return module_kallsyms_lookup_name(name); > > Should that be MODULE_NAME_LEN - 1 ? The ':' character _follows_ the module name, which can itself be upto MODULE_NAME_LEN - 1 characters. So, we should look for it till MODULE_NAME_LEN. Thanks for the review, - Naveen
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes: > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index 6a3b249a2ae1..d134b060564f 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -205,6 +205,12 @@ unsigned long kallsyms_lookup_name(const char *name) > unsigned long i; > unsigned int off; > > + if (!name || *name == '\0') > + return false; > + > + if (strnchr(name, MODULE_NAME_LEN, ':')) > + return module_kallsyms_lookup_name(name); > + > for (i = 0, off = 0; i < kallsyms_num_syms; i++) { > off = kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf)); ... } return module_kallsyms_lookup_name(name); Is the rest of the context. Which looks a bit odd, we already did module lookup previously? But it's correct, because you can lookup a symbol in a module without a module prefix, it just looks in every module. You could invert the logic, ie. check that there isn't a ":" in the name and only in that case do the for loop, always falling back to module lookup. Or just add a comment explaining why we call module lookup in two places. cheers
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c index 6a3b249a2ae1..d134b060564f 100644 --- a/kernel/kallsyms.c +++ b/kernel/kallsyms.c @@ -205,6 +205,12 @@ unsigned long kallsyms_lookup_name(const char *name) unsigned long i; unsigned int off; + if (!name || *name == '\0') + return false; + + if (strnchr(name, MODULE_NAME_LEN, ':')) + return module_kallsyms_lookup_name(name); + for (i = 0, off = 0; i < kallsyms_num_syms; i++) { off = kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
1. Fail early for invalid/zero length symbols. 2. Detect names of the form <mod:name> and skip checking for kernel symbols in that case. Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- Masami, Michael, I have added two very simple checks here, which I felt is good to have, rather than the elaborate checks in the previous version. Given the change in module code to use strnchr(), the checks are now safe and further tests are not probably not that useful. - Naveen kernel/kallsyms.c | 6 ++++++ 1 file changed, 6 insertions(+)