linux-kernel - Re: [PATCH] rss_stat: Add support to detect RSS updates of external mm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20191114164622.GC233237@google.com>
Date:   Thu, 14 Nov 2019 11:46:22 -0500
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, Ioannis Ilkos <ilkos@...gle.com>,
        minchan@...gle.com, primiano@...gle.com, fmayer@...gle.com,
        hjd@...gle.com, joaodias@...gle.com, lalitm@...gle.com,
        rslawik@...gle.com, sspatil@...gle.com, timmurray@...gle.com,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Changbin Du <changbin.du@...el.com>,
        Ingo Molnar <mingo@...hat.com>, Joe Perches <joe@...ches.com>,
        Kees Cook <keescook@...omium.org>, linux-mm@...ck.org,
        Michal Hocko <mhocko@...e.com>, Petr Mladek <pmladek@...e.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [PATCH] rss_stat: Add support to detect RSS updates of external
 mm

On Wed, Nov 13, 2019 at 03:38:16PM -0500, Steven Rostedt wrote:
[snip]
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > index dee8fc467fcf..401baaac1813 100644
> > --- a/lib/vsprintf.c
> > +++ b/lib/vsprintf.c
> > @@ -761,11 +761,34 @@ static int __init initialize_ptr_random(void)
> >  early_initcall(initialize_ptr_random);
> >  
> >  /* Maps a pointer to a 32 bit unique identifier. */
> > +int ptr_to_hashval(const void *ptr, unsigned long *hashval_out)
> > +{
> > +	const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
> > +	unsigned long hashval;
> > +
> > +	if (static_branch_unlikely(&not_filled_random_ptr_key))
> > +		return -EAGAIN;
> > +
> > +#ifdef CONFIG_64BIT
> > +	hashval = (unsigned long)siphash_1u64((u64)ptr, &ptr_key);
> > +	/*
> > +	 * Mask off the first 32 bits, this makes explicit that we have
> > +	 * modified the address (and 32 bits is plenty for a unique ID).
> > +	 */
> > +	hashval = hashval & 0xffffffff;
> > +#else
> > +	hashval = (unsigned long)siphash_1u32((u32)ptr, &ptr_key);
> > +#endif
> > +	*hashval_out = hashval;
> > +	return 0;
> > +}
> > +
> >  static char *ptr_to_id(char *buf, char *end, const void *ptr,
> >  		       struct printf_spec spec)
> >  {
> >  	const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" : "(ptrval)";
> >  	unsigned long hashval;
> > +	int ret;
> >  
> >  	/* When debugging early boot use non-cryptographically secure hash. */
> >  	if (unlikely(debug_boot_weak_hash)) {
> > @@ -773,22 +796,13 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr,
> >  		return pointer_string(buf, end, (const void *)hashval, spec);
> >  	}
> >  
> > -	if (static_branch_unlikely(&not_filled_random_ptr_key)) {
> > +	ret = ptr_to_hashval(ptr, &hashval);
> > +	if (ret) {
> 
> This is a hot path (used in trace_printk()), and you just turned a
> static_branch into a function call.
> 
> Can we make a __ptr_to_hashval() static inline, and have
> ptr_to_hashval() call that, but use the static inline here, where the
> static_branch gets called directly here?

Sure, like this?

---8<-----------------------

From: Joel Fernandes <joelaf@...gle.com>
Subject: [PATCH] vsprintf: Inline call to ptr_to_hashval

There is concern that ptr_to_hashval not being inlined can cause performance
issues (unlike before where it was a static branch) with trace_printk being a
hot path for it. Just create an inline version called __ptr_to_hashval(), and
have the actual ptr_to_hashval() call it.

Link: http://lore.kernel.org/r/20191113153816.14b95acd@gandalf.local.home
Suggested-by: Steven Rostedt <rostedt@...dmis.org>
Signed-off-by: Joel Fernandes <joelaf@...gle.com>
---
 lib/vsprintf.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 405a8ca220a0..7c488a1ce318 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -761,7 +761,7 @@ static int __init initialize_ptr_random(void)
 early_initcall(initialize_ptr_random);
 
 /* Maps a pointer to a 32 bit unique identifier. */
-int ptr_to_hashval(const void *ptr, unsigned long *hashval_out)
+static inline int __ptr_to_hashval(const void *ptr, unsigned long *hashval_out)
 {
 	unsigned long hashval;
 
@@ -782,6 +782,11 @@ int ptr_to_hashval(const void *ptr, unsigned long *hashval_out)
 	return 0;
 }
 
+int ptr_to_hashval(const void *ptr, unsigned long *hashval_out)
+{
+	return __ptr_to_hashval(ptr, hashval_out);
+}
+
 static char *ptr_to_id(char *buf, char *end, const void *ptr,
 		       struct printf_spec spec)
 {
@@ -795,7 +800,7 @@ static char *ptr_to_id(char *buf, char *end, const void *ptr,
 		return pointer_string(buf, end, (const void *)hashval, spec);
 	}
 
-	ret = ptr_to_hashval(ptr, &hashval);
+	ret = __ptr_to_hashval(ptr, &hashval);
 	if (ret) {
 		spec.field_width = 2 * sizeof(ptr);
 		/* string length must be less than default_width */
-- 
2.24.0.rc1.363.gb1bccd3e3d-goog