kernel-hardening - Re: [PATCH 2/3] vsprintf: print <no-symbol> if symbol not found

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171218010426.GY2191@eros>
Date: Mon, 18 Dec 2017 12:04:26 +1100
From: "Tobin C. Harding" <me@...in.cc>
To: Joe Perches <joe@...ches.com>
Cc: kernel-hardening@...ts.openwall.com,
	Steven Rostedt <rostedt@...dmis.org>,
	Tycho Andersen <tycho@...ho.ws>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Kees Cook <keescook@...omium.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Masahiro Yamada <yamada.masahiro@...ionext.com>,
	Alexei Starovoitov <ast@...nel.org>, linux-kernel@...r.kernel.org,
	Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH 2/3] vsprintf: print <no-symbol> if symbol not found

On Sun, Dec 17, 2017 at 04:04:14PM -0800, Joe Perches wrote:
> On Mon, 2017-12-18 at 10:53 +1100, Tobin C. Harding wrote:
> > Depends on: commit bd6b239cdbb2 ("kallsyms: don't leak address when
> > symbol not found")
> > 
> > Currently vsprintf for specifiers %p[SsB] relies on the behaviour of
> > kallsyms (sprint_symbol()) and prints the actual address if a symbol is
> > not found. Previous patch changes this behaviour so tha sprint_symbol()
> 
> tha->that
> 
> > returns an error if symbol not found. With this patch in place we can
> > print a sanitized message '<no-symbol>' instead of leaking the address.
> > 
> > Print '<no-symbol>' for printk specifier %s[sSB] if no symbol is found.
> 
> %s->%ps
> 
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> []
> > @@ -460,6 +460,8 @@ char *kvasprintf(gfp_t gfp, const char *fmt, va_list args);
> >  extern __printf(2, 0)
> >  const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
> >  
> > +extern int string_is_no_symbol(const char *s);
> > +
> []
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > 
> > +#define PRINTK_NO_SYMBOL_STR "<no-symbol>"
> 
> "<symbol unavailable>" ?  "not found"?

<symbol not found>?

> []
> 
> > +int string_is_no_symbol(const char *s)
> > +{
> > +	return !!strstr(s, PRINTK_NO_SYMBOL_STR);
> > +}
> > +EXPORT_SYMBOL(string_is_no_symbol);
> 
> Why should string_is_no_symbol be exported?

I ummed and ahhed about this. By your comment I'm guessing I made the
wrong choice. The idea behind exporting the symbol was so users of
vsprintf could have a way to see if the symbol was found or not without
having to know what string was actually printed.

I'll remove it for v3 and implement your other suggestions.

thanks for the review,
Tobin.

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.