| Message ID | 1513554812-13014-2-git-send-email-me@tobin.cc |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | kallsyms: don't leak address | expand |
On 2017-12-18 00:53, Tobin C. Harding wrote: > Currently if kallsyms_lookup() fails to find the symbol then the address > is printed. This potentially leaks sensitive information. Instead of > printing the address we can return an error, giving the calling code the > option to print the address or print some sanitized message. > > Return error instead of printing address to argument buffer. Leave > buffer in a sane state. > > Signed-off-by: Tobin C. Harding <me@tobin.cc> I think there should be a way to keep the old behavior for debugging. - Felix
On Mon, Dec 18, 2017, at 20:55, Felix Fietkau wrote: > On 2017-12-18 00:53, Tobin C. Harding wrote: > > Currently if kallsyms_lookup() fails to find the symbol then the address > > is printed. This potentially leaks sensitive information. Instead of > > printing the address we can return an error, giving the calling code the > > option to print the address or print some sanitized message. > > > > Return error instead of printing address to argument buffer. Leave > > buffer in a sane state. > > > > Signed-off-by: Tobin C. Harding <me@tobin.cc> > I think there should be a way to keep the old behavior for debugging. That was the intended use of EXPORT_SYMBOL(string_is_no_symbol); in patch 2 of this series. Then if debugging behaviour is adversely effected one could use string_is_no_symbol() on a case by case basis to add back in the original behaviour. Current suggestion on list is to remove this function. Do you have a use case in mind where debugging will break? We could add a fix to this series if so. Otherwise next version will likely drop string_is_no_symbol() thanks, Tobin.
On Tue, 19 Dec 2017 09:41:29 +1100 "Tobin C. Harding" <me@tobin.cc> wrote: > Current suggestion on list is to remove this function. Do you have a use > case in mind where debugging will break? We could add a fix to this > series if so. Otherwise next version will likely drop > string_is_no_symbol() What about adding a kernel command line parameter that lets one put back the old behavior. "insecure_print_all_symbols" ? -- Steve
On Mon, Dec 18, 2017 at 06:43:24PM -0500, Steven Rostedt wrote: > On Tue, 19 Dec 2017 09:41:29 +1100 > "Tobin C. Harding" <me@tobin.cc> wrote: > > > Current suggestion on list is to remove this function. Do you have a use > > case in mind where debugging will break? We could add a fix to this > > series if so. Otherwise next version will likely drop > > string_is_no_symbol() > > What about adding a kernel command line parameter that lets one put > back the old behavior. > > "insecure_print_all_symbols" ? Cool. I've not done that before it will be a good learning experience. I'll hack it up and see what people think. thanks, Tobin.
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c index d5fa4116688a..23b9336c1461 100644 --- a/kernel/kallsyms.c +++ b/kernel/kallsyms.c @@ -394,8 +394,10 @@ static int __sprint_symbol(char *buffer, unsigned long address, address += symbol_offset; name = kallsyms_lookup(address, &size, &offset, &modname, buffer); - if (!name) - return sprintf(buffer, "0x%lx", address - symbol_offset); + if (!name) { + buffer[0] = '\0'; + return -1; + } if (name != buffer) strcpy(buffer, name);
Currently if kallsyms_lookup() fails to find the symbol then the address is printed. This potentially leaks sensitive information. Instead of printing the address we can return an error, giving the calling code the option to print the address or print some sanitized message. Return error instead of printing address to argument buffer. Leave buffer in a sane state. Signed-off-by: Tobin C. Harding <me@tobin.cc> --- kernel/kallsyms.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)