Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1471055
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc |
| Date | 2016-08-27 02:40 +0200 |
| Message-ID | <sazPb-5US-7@gated-at.bofh.it> (permalink) |
| References | (4 earlier) <saf46-Zl-9@gated-at.bofh.it> <sagtc-1KQ-9@gated-at.bofh.it> <sapG9-7Ij-9@gated-at.bofh.it> <sapPQ-7Lt-15@gated-at.bofh.it> <sawxX-3Rz-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
[Multipart message — attachments visible in raw view] - view raw
On Fri, Aug 26, 2016 at 1:56 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> There's one problem with that though. It's going to annoy a lot of
> people who do allyesconfig/allmodconfig builds because
> DEBUG_STRICT_USER_COPY_CHECKS adds several fake warnings.
How bad is it?
In particular, we've definitely had issues with the "warning"
attribute before. Because as you pointed out somewhere elsewhere, the
warrning can happen before the call is actually optimized away by a
later compiler phase.
In particular, we _have_ occasionally fixed this by turning it into a
link-time error instead (that's in fact the really traditional model).
That makes the errior happen much later, and the error message isn't
nearly as nice (you get something like "undefined reference to unknown
symbol '__copy_to_user_failed' in function xyz" without line numbers
etc nice things). But it cuts down on the false positives that come
from warnings triggering before the final code has actually been
generated.
So one option *might* be to make the copy_to_user checks do an
explicitly constant and static check like
if (__builtin_constant_p(n) && sz >= 0 && n > sz)
__copy_to_user_failed();
with the "__copy_to_user_failed()" function declared but never
defined. That way, at link time, if something still references it, you
get a link error and you'll know it's bad.
So something like the attached patch *might* work. As mentioned, it
makes the error messages much less legible if they happen, and it
delays them to link time, so it's not perfect. But it certainly has
the potential of avoiding bogus warnings.
It *seemed* to work in my quick allmodconfig build test, but that may
be because I screwed something up. So take that with a large pinch of
salt.
What do people think? The static built-time errors - if they happen -
really should be pretty exceptional and unusual. So maybe it's ok that
they then would be somewhat cryptic, and you'd have to maybe hunt
(possibly through several layers of inline functions) where the actual
offending user copy then ends up being..
So I'm not happy with this patch, but I also think that the false
positives make the *current* code simply unworkable with current gcc
versions.
Of course, somebody might be able to come up with a better approach
that still gets the nice error messages and avoids the false
positives.
Linus
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-25 23:00 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Kees Cook <keescook@chromium.org> - 2016-08-26 04:30 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-26 06:00 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Kees Cook <keescook@chromium.org> - 2016-08-26 15:50 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-26 16:00 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-26 23:10 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-26 23:10 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Linus Torvalds <torvalds@linux-foundation.org> - 2016-08-27 02:40 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-29 16:50 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Linus Torvalds <torvalds@linux-foundation.org> - 2016-08-29 17:40 +0200
[PATCH v2] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-29 19:10 +0200
Re: [PATCH v2] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-29 20:10 +0200
[PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-30 15:10 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Linus Torvalds <torvalds@linux-foundation.org> - 2016-08-30 19:10 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Kees Cook <keescook@chromium.org> - 2016-08-30 20:20 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Josh Poimboeuf <jpoimboe@redhat.com> - 2016-08-30 21:10 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Kees Cook <keescook@chromium.org> - 2016-08-30 21:30 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Al Viro <viro@ZenIV.linux.org.uk> - 2016-08-30 22:20 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Kees Cook <keescook@chromium.org> - 2016-08-31 00:30 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Mark Rutland <mark.rutland@arm.com> - 2016-08-31 11:50 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Linus Torvalds <torvalds@linux-foundation.org> - 2016-08-30 20:20 +0200
Re: [PATCH v3] mm/usercopy: get rid of CONFIG_DEBUG_STRICT_USER_COPY_CHECKS Al Viro <viro@ZenIV.linux.org.uk> - 2016-08-30 20:20 +0200
Re: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc Kees Cook <keescook@chromium.org> - 2016-08-30 20:40 +0200
csiph-web