Linux-KBuild Archive mirror
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: Kees Cook <keescook@chromium.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>,
	linux-kbuild@vger.kernel.org,  linux-arch@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	 Andrey Ryabinin <ryabinin.a.a@gmail.com>,
	Alexander Potapenko <glider@google.com>,
	 Andrey Konovalov <andreyknvl@gmail.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	 Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	 Peter Zijlstra <peterz@infradead.org>,
	Peter Oberparleiter <oberpar@linux.ibm.com>,
	 Roberto Sassu <roberto.sassu@huaweicloud.com>,
	Johannes Berg <johannes@sipsolutions.net>,
	 kasan-dev@googlegroups.com, linux-hardening@vger.kernel.org
Subject: Re: [PATCH 0/3] kbuild: remove many tool coverage variables
Date: Mon, 13 May 2024 21:54:38 +0200	[thread overview]
Message-ID: <CANpmjNO=v=CV2Z_PGFu6ChfALiWJo3CJBDnWqUdqobO5X_62cA@mail.gmail.com> (raw)
In-Reply-To: <202405131136.73E766AA8@keescook>

On Mon, 13 May 2024 at 20:48, Kees Cook <keescook@chromium.org> wrote:
>
> In the future can you CC the various maintainers of the affected
> tooling? :)
>
> On Mon, May 06, 2024 at 10:35:41PM +0900, Masahiro Yamada wrote:
> >
> > This patch set removes many instances of the following variables:
> >
> >   - OBJECT_FILES_NON_STANDARD
> >   - KASAN_SANITIZE
> >   - UBSAN_SANITIZE
> >   - KCSAN_SANITIZE
> >   - KMSAN_SANITIZE
> >   - GCOV_PROFILE
> >   - KCOV_INSTRUMENT
> >
> > Such tools are intended only for kernel space objects, most of which
> > are listed in obj-y, lib-y, or obj-m.

I welcome the simplification, but see below.

> This is a reasonable assertion, and the changes really simplify things
> now and into the future. Thanks for finding such a clean solution! I
> note that it also immediately fixes the issue noticed and fixed here:
> https://lore.kernel.org/all/20240513122754.1282833-1-roberto.sassu@huaweicloud.com/
>
> > The best guess is, objects in $(obj-y), $(lib-y), $(obj-m) can opt in
> > such tools. Otherwise, not.
> >
> > This works in most places.
>
> I am worried about the use of "guess" and "most", though. :) Before, we
> had some clear opt-out situations, and now it's more of a side-effect. I
> think this is okay, but I'd really like to know more about your testing.
>
> It seems like you did build testing comparing build flags, since you
> call out some of the explicit changes in patch 2, quoting:
>
> >  - include arch/mips/vdso/vdso-image.o into UBSAN, GCOV, KCOV
> >  - include arch/sparc/vdso/vdso-image-*.o into UBSAN
> >  - include arch/sparc/vdso/vma.o into UBSAN
> >  - include arch/x86/entry/vdso/extable.o into KASAN, KCSAN, UBSAN, GCOV, KCOV
> >  - include arch/x86/entry/vdso/vdso-image-*.o into KASAN, KCSAN, UBSAN, GCOV, KCOV
> >  - include arch/x86/entry/vdso/vdso32-setup.o into KASAN, KCSAN, UBSAN, GCOV, KCOV
> >  - include arch/x86/entry/vdso/vma.o into GCOV, KCOV
> >  - include arch/x86/um/vdso/vma.o into KASAN, GCOV, KCOV
>
> I would agree that these cases are all likely desirable.
>
> Did you find any cases where you found that instrumentation was _removed_
> where not expected?

In addition, did you boot test these kernels? While I currently don't
recall if the vdso code caused us problems (besides the linking
problem for non-kernel objects), anything that is opted out from
instrumentation in arch/ code needs to be carefully tested if it
should be opted back into instrumentation. We had many fun hours
debugging boot hangs or other recursion issues due to instrumented
arch code.

Thanks,
-- Marco

  reply	other threads:[~2024-05-13 19:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-06 13:35 [PATCH 0/3] kbuild: remove many tool coverage variables Masahiro Yamada
2024-05-06 13:35 ` [PATCH 1/3] kbuild: provide reasonable defaults for tool coverage Masahiro Yamada
2024-05-28 11:35   ` Arnd Bergmann
2024-05-31  8:52     ` Masahiro Yamada
2024-05-31  9:05       ` Arnd Bergmann
2024-05-31 10:16         ` Masahiro Yamada
2024-05-31 16:09           ` Kees Cook
2024-05-06 13:35 ` [PATCH 2/3] Makefile: remove redundant tool coverage variables Masahiro Yamada
2024-05-06 13:35 ` [PATCH 3/3] kbuild: use GCOV_PROFILE and KCSAN_SANITIZE in scripts/Makefile.modfinal Masahiro Yamada
2024-05-13 18:48 ` [PATCH 0/3] kbuild: remove many tool coverage variables Kees Cook
2024-05-13 19:54   ` Marco Elver [this message]
2024-05-13 22:50     ` Masahiro Yamada
2024-05-13 22:39   ` Masahiro Yamada
2024-05-13 23:28     ` Kees Cook
2024-05-14  7:31   ` Roberto Sassu

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='CANpmjNO=v=CV2Z_PGFu6ChfALiWJo3CJBDnWqUdqobO5X_62cA@mail.gmail.com' \
    --to=elver@google.com \
    --cc=andreyknvl@gmail.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=johannes@sipsolutions.net \
    --cc=jpoimboe@kernel.org \
    --cc=kasan-dev@googlegroups.com \
    --cc=keescook@chromium.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=oberpar@linux.ibm.com \
    --cc=peterz@infradead.org \
    --cc=roberto.sassu@huaweicloud.com \
    --cc=ryabinin.a.a@gmail.com \
    --cc=vincenzo.frascino@arm.com \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).