From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Petr Mladek <pmladek@suse.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
"Tobin C . Harding" <me@tobin.cc>, Joe Perches <joe@perches.com>,
Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@suse.cz>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 01/11] vsprintf: Shuffle misc pointer to string functions
Date: Wed, 25 Apr 2018 17:57:09 +0300 [thread overview]
Message-ID: <1524668229.21176.558.camel@linux.intel.com> (raw)
In-Reply-To: <20180425111251.13246-2-pmladek@suse.com>
On Wed, 2018-04-25 at 13:12 +0200, Petr Mladek wrote:
> This is just a preparation step for further changes.
>
> The patch does not change the code.
>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
> lib/vsprintf.c | 244 ++++++++++++++++++++++++++++------------------
> -----------
> 1 file changed, 122 insertions(+), 122 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index b82f0c6c2aec..19fdfe621b40 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -613,6 +613,128 @@ char *string(char *buf, char *end, const char
> *s, struct printf_spec spec)
> }
>
> static noinline_for_stack
> +char *pointer_string(char *buf, char *end, const void *ptr,
> + struct printf_spec spec)
> +{
> + spec.base = 16;
> + spec.flags |= SMALL;
> + if (spec.field_width == -1) {
> + spec.field_width = 2 * sizeof(ptr);
> + spec.flags |= ZEROPAD;
> + }
> +
> + return number(buf, end, (unsigned long int)ptr, spec);
> +}
> +
> +static bool have_filled_random_ptr_key __read_mostly;
> +static siphash_key_t ptr_key __read_mostly;
> +
> +static void fill_random_ptr_key(struct random_ready_callback *unused)
> +{
> + get_random_bytes(&ptr_key, sizeof(ptr_key));
> + /*
> + * have_filled_random_ptr_key==true is dependent on
> get_random_bytes().
> + * ptr_to_id() needs to see have_filled_random_ptr_key==true
> + * after get_random_bytes() returns.
> + */
> + smp_mb();
> + WRITE_ONCE(have_filled_random_ptr_key, true);
> +}
> +
> +static struct random_ready_callback random_ready = {
> + .func = fill_random_ptr_key
> +};
> +
> +static int __init initialize_ptr_random(void)
> +{
> + int ret = add_random_ready_callback(&random_ready);
> +
> + if (!ret) {
> + return 0;
> + } else if (ret == -EALREADY) {
> + fill_random_ptr_key(&random_ready);
> + return 0;
> + }
> +
> + return ret;
> +}
> +early_initcall(initialize_ptr_random);
> +
> +/* Maps a pointer to a 32 bit unique identifier. */
> +static char *ptr_to_id(char *buf, char *end, void *ptr, struct
> printf_spec spec)
> +{
> + const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" :
> "(ptrval)";
> + unsigned long hashval;
> +
> + if (unlikely(!have_filled_random_ptr_key)) {
> + spec.field_width = 2 * sizeof(ptr);
> + /* string length must be less than default_width */
> + return string(buf, end, str, spec);
> + }
> +
> +#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
> + return pointer_string(buf, end, (const void *)hashval, spec);
> +}
> +
> +int kptr_restrict __read_mostly;
> +
> +static noinline_for_stack
> +char *restricted_pointer(char *buf, char *end, const void *ptr,
> + struct printf_spec spec)
> +{
> + switch (kptr_restrict) {
> + case 0:
> + /* Always print %pK values */
> + break;
> + case 1: {
> + const struct cred *cred;
> +
> + /*
> + * kptr_restrict==1 cannot be used in IRQ context
> + * because its test for CAP_SYSLOG would be
> meaningless.
> + */
> + if (in_irq() || in_serving_softirq() || in_nmi()) {
> + if (spec.field_width == -1)
> + spec.field_width = 2 * sizeof(ptr);
> + return string(buf, end, "pK-error", spec);
> + }
> +
> + /*
> + * Only print the real pointer value if the current
> + * process has CAP_SYSLOG and is running with the
> + * same credentials it started with. This is because
> + * access to files is checked at open() time, but %pK
> + * checks permission at read() time. We don't want to
> + * leak pointer values if a binary opens a file using
> + * %pK and then elevates privileges before reading
> it.
> + */
> + cred = current_cred();
> + if (!has_capability_noaudit(current, CAP_SYSLOG) ||
> + !uid_eq(cred->euid, cred->uid) ||
> + !gid_eq(cred->egid, cred->gid))
> + ptr = NULL;
> + break;
> + }
> + case 2:
> + default:
> + /* Always print 0's for %pK */
> + ptr = NULL;
> + break;
> + }
> +
> + return pointer_string(buf, end, ptr, spec);
> +}
> +
> +static noinline_for_stack
> char *dentry_name(char *buf, char *end, const struct dentry *d,
> struct printf_spec spec,
> const char *fmt)
> {
> @@ -1358,69 +1480,6 @@ char *uuid_string(char *buf, char *end, const
> u8 *addr,
> }
>
> static noinline_for_stack
> -char *pointer_string(char *buf, char *end, const void *ptr,
> - struct printf_spec spec)
> -{
> - spec.base = 16;
> - spec.flags |= SMALL;
> - if (spec.field_width == -1) {
> - spec.field_width = 2 * sizeof(ptr);
> - spec.flags |= ZEROPAD;
> - }
> -
> - return number(buf, end, (unsigned long int)ptr, spec);
> -}
> -
> -int kptr_restrict __read_mostly;
> -
> -static noinline_for_stack
> -char *restricted_pointer(char *buf, char *end, const void *ptr,
> - struct printf_spec spec)
> -{
> - switch (kptr_restrict) {
> - case 0:
> - /* Always print %pK values */
> - break;
> - case 1: {
> - const struct cred *cred;
> -
> - /*
> - * kptr_restrict==1 cannot be used in IRQ context
> - * because its test for CAP_SYSLOG would be
> meaningless.
> - */
> - if (in_irq() || in_serving_softirq() || in_nmi()) {
> - if (spec.field_width == -1)
> - spec.field_width = 2 * sizeof(ptr);
> - return string(buf, end, "pK-error", spec);
> - }
> -
> - /*
> - * Only print the real pointer value if the current
> - * process has CAP_SYSLOG and is running with the
> - * same credentials it started with. This is because
> - * access to files is checked at open() time, but %pK
> - * checks permission at read() time. We don't want to
> - * leak pointer values if a binary opens a file using
> - * %pK and then elevates privileges before reading
> it.
> - */
> - cred = current_cred();
> - if (!has_capability_noaudit(current, CAP_SYSLOG) ||
> - !uid_eq(cred->euid, cred->uid) ||
> - !gid_eq(cred->egid, cred->gid))
> - ptr = NULL;
> - break;
> - }
> - case 2:
> - default:
> - /* Always print 0's for %pK */
> - ptr = NULL;
> - break;
> - }
> -
> - return pointer_string(buf, end, ptr, spec);
> -}
> -
> -static noinline_for_stack
> char *netdev_bits(char *buf, char *end, const void *addr, const char
> *fmt)
> {
> unsigned long long num;
> @@ -1654,65 +1713,6 @@ char *device_node_string(char *buf, char *end,
> struct device_node *dn,
> return widen_string(buf, buf - buf_start, end, spec);
> }
>
> -static bool have_filled_random_ptr_key __read_mostly;
> -static siphash_key_t ptr_key __read_mostly;
> -
> -static void fill_random_ptr_key(struct random_ready_callback *unused)
> -{
> - get_random_bytes(&ptr_key, sizeof(ptr_key));
> - /*
> - * have_filled_random_ptr_key==true is dependent on
> get_random_bytes().
> - * ptr_to_id() needs to see have_filled_random_ptr_key==true
> - * after get_random_bytes() returns.
> - */
> - smp_mb();
> - WRITE_ONCE(have_filled_random_ptr_key, true);
> -}
> -
> -static struct random_ready_callback random_ready = {
> - .func = fill_random_ptr_key
> -};
> -
> -static int __init initialize_ptr_random(void)
> -{
> - int ret = add_random_ready_callback(&random_ready);
> -
> - if (!ret) {
> - return 0;
> - } else if (ret == -EALREADY) {
> - fill_random_ptr_key(&random_ready);
> - return 0;
> - }
> -
> - return ret;
> -}
> -early_initcall(initialize_ptr_random);
> -
> -/* Maps a pointer to a 32 bit unique identifier. */
> -static char *ptr_to_id(char *buf, char *end, void *ptr, struct
> printf_spec spec)
> -{
> - const char *str = sizeof(ptr) == 8 ? "(____ptrval____)" :
> "(ptrval)";
> - unsigned long hashval;
> -
> - if (unlikely(!have_filled_random_ptr_key)) {
> - spec.field_width = 2 * sizeof(ptr);
> - /* string length must be less than default_width */
> - return string(buf, end, str, spec);
> - }
> -
> -#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
> - return pointer_string(buf, end, (const void *)hashval, spec);
> -}
> -
> /*
> * Show a '%p' thing. A kernel extension is that the '%p' is
> followed
> * by an extra set of alphanumeric characters that are extended
> format
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
next prev parent reply other threads:[~2018-04-25 14:57 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-25 11:12 [PATCH v5 00/11] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
2018-04-25 11:12 ` [PATCH v5 01/11] vsprintf: Shuffle misc pointer to string functions Petr Mladek
2018-04-25 14:57 ` Andy Shevchenko [this message]
2018-04-25 11:12 ` [PATCH v5 02/11] vsprintf: Add missing const ptr qualifier to prt_to_id() Petr Mladek
2018-04-25 14:57 ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 03/11] vsprintf: Consistent %pK handling for kptr_restrict == 0 Petr Mladek
2018-04-25 14:58 ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 04/11] vsprintf: Do not check address of well-known strings Petr Mladek
2018-04-25 11:44 ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 05/11] vsprintf: Consolidate handling of unknown pointer specifiers Petr Mladek
2018-04-25 13:08 ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 06/11] vsprintf: Factor out %p[iI] handler as ip_addr_string() Petr Mladek
2018-04-25 13:11 ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 07/11] vsprintf: Factor out %pV handler as va_format() Petr Mladek
2018-04-25 14:56 ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 08/11] vsprintf: Factor out %pO handler as kobject_string() Petr Mladek
2018-04-25 15:01 ` Andy Shevchenko
2018-04-25 11:12 ` [PATCH v5 09/11] vsprintf: Prevent crash when dereferencing invalid pointers Petr Mladek
2018-04-25 15:10 ` Andy Shevchenko
2018-04-25 15:32 ` Andy Shevchenko
2018-04-27 12:47 ` Petr Mladek
2018-05-03 11:55 ` Andy Shevchenko
2018-04-26 21:46 ` kbuild test robot
2018-04-25 11:12 ` [PATCH v5 10/11] vsprintf: WARN() on invalid pointer access Petr Mladek
2018-04-25 12:43 ` Rasmus Villemoes
2018-04-26 1:28 ` Sergey Senozhatsky
2018-04-27 12:37 ` Petr Mladek
2018-04-25 11:12 ` [PATCH v5 11/11] vsprintf: Avoid confusion between invalid address and value Petr Mladek
2018-04-27 14:10 ` [PATCH v5 00/11] vsprintf: Prevent silent crashes and consolidate error handling Petr Mladek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1524668229.21176.558.camel@linux.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=me@tobin.cc \
--cc=mhocko@suse.cz \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=sergey.senozhatsky.work@gmail.com \
--cc=sergey.senozhatsky@gmail.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).