mbox series

[GIT,PULL] FORTIFY_SOURCE updates for v5.18-rc1

Message ID 202203251443.9BBADFD98@keescook (mailing list archive)
State Mainlined
Headers show
Series [GIT,PULL] FORTIFY_SOURCE updates for v5.18-rc1 | expand

Pull-request

https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/memcpy-v5.18-rc1

Message

Kees Cook March 25, 2022, 10:03 p.m. UTC
Hi Linus,

Please pull these FORTIFY_SOURCE updates for v5.18-rc1.

This series consists of two halves:

- strict compile-time buffer size checking under FORTIFY_SOURCE for
  the memcpy()-family of functions (for extensive details and rationale,
  see the first commit),

- enabling FORTIFY_SOURCE for Clang, which has had many overlapping bugs
  that we've finally worked past.

It looks like all the dependent trees with related buffer fixes have been
merged (I was waiting for the scsi tree to get pulled). This has been
in -next for almost 2 development cycles, and I did overnight build
testing merged against your tree under the following combinations,
with no new warnings (there is one Clang 14+ specific issue in
drivers/net/ethernet/huawei/hinic that we're still tracking down as a
likely compiler regression[1]):

gcc   11.2.1 (Fedora 35)    defconfig:    x86_64 i386 arm64
gcc   11.2.1 (Fedora 35)    allmodconfig: x86_64 i386 arm64
gcc   11.2.0 (Ubuntu 21.10) defconfig:    x86_64 i386 arm64
gcc   11.2.0 (Ubuntu 21.10) allmodconfig: x86_64 i386 arm64
gcc   10.3.0 (Ubuntu 21.10) defconfig:    x86_64 i386 arm64
gcc   10.3.0 (Ubuntu 21.10) allmodconfig: x86_64 i386 arm64
gcc    9.4.0 (Ubuntu 21.10) defconfig:    x86_64 i386 arm64
gcc    9.4.0 (Ubuntu 21.10) allmodconfig: x86_64 i386 arm64
gcc    8.5.0 (Ubuntu 21.10) defconfig:    x86_64 i386 arm64
gcc    8.5.0 (Ubuntu 21.10) allmodconfig: x86_64 i386 arm64
clang 15.0.0 (local build)  defconfig:    x86_64 i386 arm64
clang 14.0.0 (Ubuntu 22.04) defconfig:    x86_64 i386 arm64
clang 13.0.0 (Fedora 35)    defconfig:    x86_64 i386 arm64
clang 12.0.1 (Ubuntu 21.10) defconfig:    x86_64 i386 arm64
clang 13.0.0 (Ubuntu 21.10) allmodconfig: x86_64 i386
clang 12.0.1 (Ubuntu 21.10) allmodconfig: x86_64 i386

There is also still 1 runtime fix pending for the comedi driver's
selftests[2], which is living in my "pending-fixes" tree (for fixes that
maintainers appear to have picked up, but haven't appeared in -next yet).

Beyond that, as far as I've been able to track, all the other architecture
also build cleanly; we've been fixing any issues as they are reported
by various builders, and when we find them in our builds.

Thanks!

-Kees

[1] https://github.com/ClangBuiltLinux/linux/issues/1592
[2] https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=for-next/pending-fixes&id=77cc24d543c46076d753348b6178473eb16fc788

The following changes since commit dfd42facf1e4ada021b939b4e19c935dcdd55566:

  Linux 5.17-rc3 (2022-02-06 12:20:50 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/memcpy-v5.18-rc1

for you to fetch changes up to 281d0c962752fb40866dd8d4cade68656f34bd1f:

  fortify: Add Clang support (2022-02-13 16:50:07 -0800)

----------------------------------------------------------------
memcpy updates for v5.18-rc1

- Enable strict FORTIFY_SOURCE compile-time validation of memcpy buffers

- Add Clang features needed for FORTIFY_SOURCE support

- Enable FORTIFY_SOURCE for Clang where possible

----------------------------------------------------------------
Kees Cook (12):
      fortify: Detect struct member overflows in memcpy() at compile-time
      fortify: Detect struct member overflows in memmove() at compile-time
      fortify: Detect struct member overflows in memset() at compile-time
      fortify: Update compile-time tests for Clang 14
      fortify: Replace open-coded __gnu_inline attribute
      Compiler Attributes: Add __pass_object_size for Clang
      Compiler Attributes: Add __overloadable for Clang
      Compiler Attributes: Add __diagnose_as for Clang
      fortify: Make pointer arguments const
      fortify: Use __diagnose_as() for better diagnostic coverage
      fortify: Make sure strlen() may still be used as a constant expression
      fortify: Add Clang support

 arch/x86/boot/compressed/misc.c                 |   3 +-
 arch/x86/lib/memcpy_32.c                        |   1 +
 include/linux/compiler_attributes.h             |  39 ++++
 include/linux/fortify-string.h                  | 238 +++++++++++++++++++-----
 lib/Makefile                                    |   3 +-
 lib/string_helpers.c                            |   6 +
 lib/test_fortify/read_overflow2_field-memcpy.c  |   5 +
 lib/test_fortify/read_overflow2_field-memmove.c |   5 +
 lib/test_fortify/write_overflow_field-memcpy.c  |   5 +
 lib/test_fortify/write_overflow_field-memmove.c |   5 +
 lib/test_fortify/write_overflow_field-memset.c  |   5 +
 scripts/test_fortify.sh                         |   8 +-
 security/Kconfig                                |   5 +-
 13 files changed, 272 insertions(+), 56 deletions(-)
 create mode 100644 lib/test_fortify/read_overflow2_field-memcpy.c
 create mode 100644 lib/test_fortify/read_overflow2_field-memmove.c
 create mode 100644 lib/test_fortify/write_overflow_field-memcpy.c
 create mode 100644 lib/test_fortify/write_overflow_field-memmove.c
 create mode 100644 lib/test_fortify/write_overflow_field-memset.c

Comments

Linus Torvalds March 26, 2022, 7:29 p.m. UTC | #1
On Fri, Mar 25, 2022 at 3:03 PM Kees Cook <keescook@chromium.org> wrote:
>
> It looks like all the dependent trees with related buffer fixes have been
> merged (I was waiting for the scsi tree to get pulled). This has been
> in -next for almost 2 development cycles, and I did overnight build
> testing merged against your tree under the following combinations,
> with no new warnings (there is one Clang 14+ specific issue in
> drivers/net/ethernet/huawei/hinic that we're still tracking down as a
> likely compiler regression[1]):

So how much of this is _completely_ compile-time?

Right now it looks to me like FORTIFY_SOURCE ends up doing two things

 - added runtime checking based on compile-time sizes

 - compile-time errors

How hard would it be to separate the two issues out?

Because if all the compiler issues and warnings have been sorted out,
it sounds to me like the compile-time side could/should be done
unconditionally if there are no runtime downsides.

Hmm?

                Linus
Linus Torvalds March 26, 2022, 7:40 p.m. UTC | #2
On Sat, Mar 26, 2022 at 12:29 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Because if all the compiler issues and warnings have been sorted out,
> it sounds to me like the compile-time side could/should be done
> unconditionally if there are no runtime downsides.

.. or do the existing compiler warnings for the builtins already cover
all cases, and the only reason the fortify-source code has
compile-time warnings is that the option takes over the builtins?

So maybe there's no upside to the fortify-source code for that case?

              Linus
pr-tracker-bot@kernel.org March 26, 2022, 8:18 p.m. UTC | #3
The pull request you sent on Fri, 25 Mar 2022 15:03:43 -0700:

> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/memcpy-v5.18-rc1

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/4be240b18aa67b1144af546bea2d7cad1b75c19b

Thank you!
Kees Cook March 28, 2022, 4:01 p.m. UTC | #4
On Sat, Mar 26, 2022 at 12:40:18PM -0700, Linus Torvalds wrote:
> On Sat, Mar 26, 2022 at 12:29 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Because if all the compiler issues and warnings have been sorted out,
> > it sounds to me like the compile-time side could/should be done
> > unconditionally if there are no runtime downsides.

Yeah, I'd like to do this. The way the header files are currently split
up makes this slightly weird, and there have been issues with some
arch/compiler combinations, so it's not quite as cut-and-dried as I'd
like. I'll investigate what it could look like.

> .. or do the existing compiler warnings for the builtins already cover
> all cases, and the only reason the fortify-source code has
> compile-time warnings is that the option takes over the builtins?

This mostly depends on the compiler version, and they often overlap, but
the new FORTIFY logic tends to be more strict (where possible) and is more
consistent; I view the two diagnostic capabilities as complementary.