Re: [GIT PULL] Compiler Attributes for v4.20-rc1
On 11/02/2018 07:11 PM, Linus Torvalds wrote: > On Fri, Nov 2, 2018 at 6:16 AM Andrey Ryabinin > wrote: >> >> On 11/02/2018 04:46 AM, Linus Torvalds wrote: >>> >>> So I _think_ the KASAN config should have a >>> >>> depends on CC_IS_GCC && GCC_VERSION >= 40902 >>> >>> on it, but maybe there is something I'm missing. >> >> I'd rather use cc-option instead of version check, since we also support >> clang. > > That would be even better, but I thought the requirement for 4.9.2 > came not from the option existing, but because of some bugs getting > fixed? It looks unusual but -fsanitize=kernel-address really showed up only in 4.9.2. It was actually backported from 5.0 branch to 4.9 for whatever reason. > > But if we can do it without version checks, that would be lovely. > > Linus >
Re: [GIT PULL] Compiler Attributes for v4.20-rc1
On 11/02/2018 04:46 AM, Linus Torvalds wrote: > On Thu, Nov 1, 2018 at 10:06 AM Linus Torvalds > wrote: >> >> The logic for using __no_sanitize_address *used* to be >> >> #if GCC_VERSION >= 40902 > > Ok, looking around, I think this has less to do with the attribute > being recognized, and simply just being because KASAN itself wants > gcc-4.9.2. > > I'm actually not seeing that KASAN dependency in the Kconfig scripts > (and it probably _should_ be now that we can just add compiler version > dependencies there), but that explains why the gcc version check is > different from "gcc supports the attribute". > > Anyway, I decided to do the merge by just getting rid of the > GCC_VERSION check around __no_sanitize_address_or_inline entirely. If > you enable KASAN, then a function with that marking just won't be > marked inline. > > End result: pulled. I'm as confused as you are as to why > __no_sanitize_address_or_inline is in the gcc header, but I guess it > ends up being the same issue: KASAN depends on gcc even if that > dependency doesn't seem to be spelled out in lib/Kconfig.kasan. > > So I _think_ the KASAN config should have a > > depends on CC_IS_GCC && GCC_VERSION >= 40902 > > on it, but maybe there is something I'm missing. > I'd rather use cc-option instead of version check, since we also support clang. It should be possible to express compiler requirements for subfeatures like stack/inline instrumentation in Kconfig. But that would be not that trivial, see the scripts/Makefile.kasan > But from a pull standpoint, I don't want to mess with those > (unrelated) issues, so I just kept the merge resolution as simple and > straightforward as possible. > > Miguel, please do double-check the merge (it's not pushed out yet, I'm > doing the usual build tests etc first). > > Linus >
Re: [GIT PULL] Compiler Attributes for v4.20-rc1
On 11/01/2018 08:06 PM, Linus Torvalds wrote: > On Mon, Oct 22, 2018 at 3:59 AM Miguel Ojeda > wrote: >> >> Here it is the Compiler Attributes series/tree, which tries to disentangle >> the include/linux/compiler*.h headers and bring them up to date. > > I've finally emptied the "normal" pull queues, and am looking at the > odd ones that I felt I needed to review more in-depth. > > This looked fine to me, and I already pulled, but when looking at the > conflict for __no_sanitize_address_or_inline, I also noticed that you > actually changed the gcc version logic. > > The logic for using __no_sanitize_address *used* to be > > #if GCC_VERSION >= 40902 > > but you have changed it to > > # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8) > > so now it's gcc-4.8+ rather than gcc-4.9.2+. As you said in other email - "this has less to do with the attribute being recognized, and simply just being because KASAN itself wants gcc-4.9.2." gcc < 4.9.2 simply doesn't have -fsanitize=kernel-address. But no_sanitize attribute originally comes with user space ASAN (-fsanitize=address) which exist in earlier GCCs like 4.8. Checking against 4.8 gcc should be fine. If we compile the kernel with gcc 4.8 it will be compiled without instrumentation, so the attribute won't have any effect. > > I'm not sure how much that matters (maybe the original check for 4.9.2 > was just a random pick by Andrey? Added to cc), but together with the > movement to that looks like it also > wouldn't want the CONFIG_KASAN tests, I wonder what the right merge > resolution would be. > > Yes, I see the resolution in linux-next, and I think that one is odd > and dubious, and now it *mixes* that old test of gcc-4.9.2 with the > different test in compiler-attributes. > > I'm _inclined_ to just do > > #ifdef CONFIG_KASAN > #define __no_sanitize_address_or_inline \ > __no_sanitize_address __maybe_unused notrace > #else > #define __no_sanitize_address_or_inline inline > #endif > > without any compiler versions at all, because I don't think it matters > (maybe we won't get address sanitation, and we will not get inlining > either, but do we care?). You're right, version checks shouldn't matter here. But __no_sanitize_address_or_inline shouldn't have been added in the first place, because we already have almost the same __no_kasan_or_inline: #ifdef CONFIG_KASAN /* * We can't declare function 'inline' because __no_sanitize_address confilcts * with inlining. Attempt to inline it may cause a build failure. * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368 * '__maybe_unused' allows us to avoid defined-but-not-used warnings. */ # define __no_kasan_or_inline __no_sanitize_address __maybe_unused #else # define __no_kasan_or_inline __always_inline #endif There are small differences like absence of notrace, but notrace could be added to the function together with __no_kasan_or_inline. And inline is *not* redefined to __always_inline only on x86 when CONFIG_OPTIMIZE_INLINING=y So there is absolutely no reason to have both __no_kasan_or_inline and __no_sanitize_address_or_inline.
Re: [PATCH v2] kasan: Support for r/w instrumentation control
On 12/13/2016 11:58 AM, Dmitry Vyukov wrote: > --- a/Documentation/dev-tools/kasan.rst > +++ b/Documentation/dev-tools/kasan.rst > @@ -40,6 +40,14 @@ similar to the following to the respective kernel Makefile: > > KASAN_SANITIZE := n > > +Sometimes it may be useful to disable instrumentation of reads, or writes > +or both for the entire kernel. For example, if binary size is a concern, > +it may be useful to disable instrumentation of reads to reduce binary size > but > +still catch more harmful bugs on writes. Or, if one is interested only in > +sanitization of a particular module and performance is a concern, she can > +disable instrumentation of both reads and writes for kernel code. > +Instrumentation can be disabled with CONFIG_KASAN_READS and > CONFIG_KASAN_WRITES. > + I don't understand this. How this can be related to modules? Configs are global. You can't just disable/enable config per module. > Error reports > -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] kasan: Support for r/w instrumentation control
On 12/13/2016 12:38 PM, Dmitry Vyukov wrote: > On Tue, Dec 13, 2016 at 10:20 AM, Andrey Ryabinin > <aryabi...@virtuozzo.com> wrote: >> >> >> On 12/13/2016 11:58 AM, Dmitry Vyukov wrote: >> >>> --- a/Documentation/dev-tools/kasan.rst >>> +++ b/Documentation/dev-tools/kasan.rst >>> @@ -40,6 +40,14 @@ similar to the following to the respective kernel >>> Makefile: >>> >>> KASAN_SANITIZE := n >>> >>> +Sometimes it may be useful to disable instrumentation of reads, or writes >>> +or both for the entire kernel. For example, if binary size is a concern, >>> +it may be useful to disable instrumentation of reads to reduce binary size >>> but >>> +still catch more harmful bugs on writes. Or, if one is interested only in >>> +sanitization of a particular module and performance is a concern, she can >>> +disable instrumentation of both reads and writes for kernel code. >>> +Instrumentation can be disabled with CONFIG_KASAN_READS and >>> CONFIG_KASAN_WRITES. >>> + >> >> I don't understand this. How this can be related to modules? Configs are >> global. >> You can't just disable/enable config per module. > > > Build everything without instrumentation. Then enable instrumentation > and do "make lib/test_kasan.ko". > Or build everything, copy out bzImage, change config, build everything again. Yeah, this is soo convenient... Seriously speaking, per-file instrumentation is absolutely irrelevant to this patch and should have been addressed from a different angle. E.g. see how UBSAN/GCOV/KCOV do that. As for this patch, I'd say only one option would be enough - KASAN_DONT_SANITIZE_READS. Nobody wants to sanitize only reads without writes, right? Writes are fewer and more dangerous. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] docs: sphinxify kasan.txt and move to dev-tools
On 08/09/2016 02:34 AM, Jonathan Corbet wrote: > No textual changes beyond formatting. > > Cc: Andrey Ryabinin <aryabi...@virtuozzo.com> > Cc: Alexander Potapenko <gli...@google.com> > Cc: Dmitry Vyukov <dvyu...@google.com> > Signed-off-by: Jonathan Corbet <cor...@lwn.net> > --- > Documentation/dev-tools/kasan.rst | 173 > ++ > Documentation/dev-tools/tools.rst | 1 + > Documentation/kasan.txt | 171 - > MAINTAINERS | 2 +- > 4 files changed, 175 insertions(+), 172 deletions(-) > create mode 100644 Documentation/dev-tools/kasan.rst > delete mode 100644 Documentation/kasan.txt > Acked-by: Andrey Ryabinin <aryabi...@virtuozzo.com> -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/10] docs: sphinxify ubsan.txt and move it to dev-tools
On 08/09/2016 02:34 AM, Jonathan Corbet wrote: > Cc: Andrey Ryabinin <aryabi...@virtuozzo.com> > Signed-off-by: Jonathan Corbet <cor...@lwn.net> > --- Acked-by: Andrey Ryabinin <aryabi...@virtuozzo.com> -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm, kasan: Update kasan docs to indicate arm64 support
On 08/16/2016 07:30 PM, Laura Abbott wrote:
> KASAN has been supported on arm64 since 39d114ddc682 ("arm64: add KASAN
> support"). Update the docs to indicate this.
>
> Signed-off-by: Laura Abbott <labb...@redhat.com>
Acked-by: Andrey Ryabinin <aryabi...@virtuozzo.com>
> ---
> Documentation/kasan.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/kasan.txt b/Documentation/kasan.txt
> index 7dd95b3..d167220 100644
> --- a/Documentation/kasan.txt
> +++ b/Documentation/kasan.txt
> @@ -12,7 +12,7 @@ KASAN uses compile-time instrumentation for checking every
> memory access,
> therefore you will need a GCC version 4.9.2 or later. GCC 5.0 or later is
> required for detection of out-of-bounds accesses to stack or global
> variables.
>
> -Currently KASAN is supported only for x86_64 architecture.
> +Currently KASAN is supported only for x86_64 and arm64 architecture.
>
> 1. Usage
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
