public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Matthias Kaehlcke <mka@chromium.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	James Y Knight <jyknight@google.com>,
	Chandler Carruth <chandlerc@google.com>,
	Stephen Hines <srhines@google.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Kees Cook <keescook@google.com>,
	Guenter Roeck <groeck@chromium.org>,
	Greg Hackmann <ghackmann@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [GIT PULL] x86/build changes for v4.17
Date: Wed, 4 Apr 2018 16:31:11 -0700	[thread overview]
Message-ID: <20180404233111.GJ87376@google.com> (raw)
In-Reply-To: <CA+55aFz+RcyPOTs3WM-FcwoQtgaotcBVqswma7qZ-3O2Fv__9Q@mail.gmail.com>

El Wed, Apr 04, 2018 at 03:39:24PM -0700 Linus Torvalds ha dit:

> On Wed, Apr 4, 2018 at 3:17 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Getting our compiler team high to look into this might affect a timely
> > (and correct ...) implementation of asm-goto and others important
> > features. Arnd, do you have another, preferably simple instance to
> > keep our compiler folks (halfway) sane?
> 
> I don't know if clang actually already gets this right or not, but as
> an example of a case where we have a semantic difference between "is
> this a constant or not", a much simpler case is in
> 
>  - arch/x86/include/asm/uaccess.h:
> 
>   /*
>    * Test whether a block of memory is a valid user space address.
>    * Returns 0 if the range is valid, nonzero otherwise.
>    */
>   static inline bool __chk_range_not_ok(unsigned long addr, unsigned
> long size, unsigned long limit)
>   {
>         /*
>          * If we have used "sizeof()" for the size,
>          * we know it won't overflow the limit (but
>          * it might overflow the 'addr', so it's
>          * important to subtract the size from the
>          * limit, not add it to the address).
>          */
>         if (__builtin_constant_p(size))
>                 return unlikely(addr > limit - size);
> 
>         /* Arbitrary sizes? Be careful about overflow */
>         addr += size;
>         if (unlikely(addr < size))
>                 return true;
>         return unlikely(addr > limit);
>   }
> 
> where the actual check itself is simplified for the constant size case
> (because we know that constant sizes are never going to have the
> overflow issue with the address size limit)
> 
> That inline function itself is then wrapped with a couple of macros,
> and the usual use-case is through "access_ok()", which then typically
> just gets either a sizeof(), or a non-constant length.
> 
> Of course, we've been walking away from having people do "access_ok()
> + __copy_from_user()" (the latter does some conceptually similar
> optimizations on constant sizes), so those probably simply don't
> matter much any more in practice.
> 
> But they are certainly a lot simpler to look at than the more exciting
> 32-bit asm-generic "do_div()" case is ;)

Thanks, that was useful!

>From some experiments it looks like clang, in difference to gcc, does
not treat constant values passed as parameters to inline function as
constants.

I'll ask our compiler folks to take a look, with lower priority than
other issues in this thread though.

  reply	other threads:[~2018-04-04 23:31 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-02  9:50 [GIT PULL] x86/build changes for v4.17 Ingo Molnar
2018-04-02 21:44 ` Linus Torvalds
2018-04-02 22:38   ` Matthias Kaehlcke
2018-04-03  1:26     ` Matthias Kaehlcke
2018-04-03  8:59   ` Peter Zijlstra
2018-04-03  9:51     ` Ingo Molnar
2018-04-03 12:09       ` Peter Zijlstra
2018-04-03 18:06       ` Matthias Kaehlcke
2018-04-03 21:58         ` Nick Desaulniers
2018-04-04  9:19           ` Peter Zijlstra
2018-04-04  9:38           ` Greg KH
2018-04-04 16:49             ` Nick Desaulniers
2018-04-04 17:13               ` Linus Torvalds
2018-04-04 17:46                 ` Nick Desaulniers
2018-04-04 23:10                 ` Nick Desaulniers
2018-04-04 16:53             ` Nick Desaulniers
2018-04-04 16:59               ` Greg KH
2018-04-04 19:26                 ` James Y Knight
2018-04-04 19:42                   ` Linus Torvalds
2018-04-04 22:21                     ` James Y Knight
2018-04-04 22:29                       ` Linus Torvalds
2018-04-05  7:08                       ` Peter Zijlstra
2018-04-05 16:21                         ` James Y Knight
2018-04-04 19:32               ` Josh Poimboeuf
2018-06-07 19:23                 ` Nick Desaulniers
2018-06-07 20:11                   ` Greg KH
2018-04-04  9:30         ` Peter Zijlstra
2018-04-04 19:17           ` Matthias Kaehlcke
2018-04-04 20:33             ` Arnd Bergmann
2018-04-04 20:58               ` Matthias Kaehlcke
2018-04-04 21:11                 ` Arnd Bergmann
2018-04-04 21:46                   ` Matthias Kaehlcke
2018-04-04 21:59                     ` Linus Torvalds
2018-04-04 22:17                       ` Matthias Kaehlcke
2018-04-04 22:39                         ` Linus Torvalds
2018-04-04 23:31                           ` Matthias Kaehlcke [this message]
2018-04-05  0:05                             ` Linus Torvalds
2018-04-05  0:20                               ` Kees Cook
2018-04-05  7:24                               ` Peter Zijlstra
2018-04-05  8:04                                 ` Ingo Molnar
2018-04-05  8:24                                   ` Peter Zijlstra
2018-04-05 16:43                                 ` Linus Torvalds
2018-04-05  7:20                             ` Peter Zijlstra
2018-04-05 17:46                               ` James Y Knight
2018-04-05 18:06                                 ` Linus Torvalds
2018-04-05 20:51                                   ` James Y Knight
2018-04-05 21:13                                     ` Linus Torvalds
2018-04-05 22:51                                       ` James Y Knight
2018-04-06  2:02                                         ` Linus Torvalds
2018-04-05 17:47                               ` James Y Knight
2018-04-04 23:04             ` Nick Desaulniers
2018-04-03 17:36     ` Linus Torvalds

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=20180404233111.GJ87376@google.com \
    --to=mka@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=chandlerc@google.com \
    --cc=ghackmann@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=groeck@chromium.org \
    --cc=jyknight@google.com \
    --cc=keescook@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    --cc=srhines@google.com \
    --cc=tglx@linutronix.de \
    --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