mbox series

[GIT,PULL] hardening updates for v6.3-rc1

Message ID 63efd7ab.170a0220.3442b.6609@mx.google.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [GIT,PULL] hardening updates for v6.3-rc1 | expand

Pull-request

https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/hardening-v6.3-rc1

Message

Kees Cook Feb. 17, 2023, 7:38 p.m. UTC
Hi Linus,

Please pull these hardening updates for v6.3-rc1. Beyond some specific
LoadPin, UBSAN, and fortify features, there are other fixes scattered
around in various subsystems where maintainers were okay with me carrying
them in my tree or were non-responsive but the patches were reviewed
by others.

Thanks!

-Kees

The following changes since commit be0d8f48ad97f5b775b0af3310343f676dbf318a:

  bcache: Silence memcpy() run-time false positive warnings (2023-01-25 12:24:50 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/hardening-v6.3-rc1

for you to fetch changes up to 78f7a3fd6dc66cb788c21d7705977ed13c879351:

  randstruct: disable Clang 15 support (2023-02-08 15:26:58 -0800)

----------------------------------------------------------------
hardening updates for v6.3-rc1

- Replace 0-length and 1-element arrays with flexible arrays in various
  subsystems (Paulo Miguel Almeida, Stephen Rothwell, Kees Cook)

- randstruct: Disable Clang 15 support (Eric Biggers)

- GCC plugins: Drop -std=gnu++11 flag (Sam James)

- strpbrk(): Refactor to use strchr() (Andy Shevchenko)

- LoadPin LSM: Allow root filesystem switching when non-enforcing

- UBSAN: Improve arm64 trap code reporting

- fortify: Use dynamic object size hints when available

- ext4: Fix CFI function prototype mismatch

- Nouveau: Fix DP buffer size arguments

- hisilicon: Wipe entire crypto DMA pool on error

- coda: Fully allocate sig_inputArgs

- copy_struct_from_user(): Add minimum bounds check on kernel buffer size

----------------------------------------------------------------
Andy Shevchenko (1):
      lib/string: Use strchr() in strpbrk()

Eric Biggers (1):
      randstruct: disable Clang 15 support

Kees Cook (15):
      fortify: Use __builtin_dynamic_object_size() when available
      ARM: ixp4xx: Replace 0-length arrays with flexible arrays
      LoadPin: Refactor read-only check into a helper
      LoadPin: Refactor sysctl initialization
      LoadPin: Move pin reporting cleanly out of locking
      LoadPin: Allow filesystem switch when not enforcing
      drm/nouveau/disp: Fix nvif_outp_acquire_dp() argument size
      ext4: Fix function prototype mismatch for ext4_feat_ktype
      io_uring: Replace 0-length array with flexible array
      net/i40e: Replace 0-length array with flexible array
      crypto: hisilicon: Wipe entire pool on error
      Merge branch 'for-linus/hardening' into for-next/hardening
      coda: Avoid partial allocation of sig_inputArgs
      arm64: Support Clang UBSAN trap codes for better reporting
      uaccess: Add minimum bounds check on kernel buffer size

Paulo Miguel Almeida (1):
      i915/gvt: Replace one-element array with flexible-array member

Sam James (1):
      gcc-plugins: drop -std=gnu++11 to fix GCC 13 build

Stephen Rothwell (1):
      rxrpc: replace zero-lenth array with DECLARE_FLEX_ARRAY() helper

 arch/arm64/include/asm/brk-imm.h            |  3 +
 arch/arm64/kernel/traps.c                   | 21 +++++++
 drivers/crypto/hisilicon/sgl.c              |  3 +-
 drivers/gpu/drm/i915/gvt/firmware.c         |  4 +-
 drivers/gpu/drm/nouveau/include/nvif/outp.h |  3 +-
 drivers/gpu/drm/nouveau/nvif/outp.c         |  2 +-
 drivers/misc/lkdtm/heap.c                   |  1 +
 drivers/net/ethernet/intel/i40e/i40e.h      |  2 +-
 drivers/soc/ixp4xx/ixp4xx-npe.c             |  6 +-
 fs/coda/upcall.c                            |  2 +-
 fs/ext4/sysfs.c                             |  7 ++-
 include/linux/compiler_attributes.h         |  5 ++
 include/linux/fortify-string.h              |  7 +++
 include/linux/uaccess.h                     |  4 ++
 include/linux/ubsan.h                       |  9 +++
 include/uapi/linux/io_uring.h               |  2 +-
 lib/Makefile                                |  2 -
 lib/string.c                                | 10 ++--
 lib/ubsan.c                                 | 68 ++++++++++++++++++++++
 lib/ubsan.h                                 | 32 +++++++++++
 net/rxrpc/ar-internal.h                     |  2 +-
 scripts/gcc-plugins/Makefile                |  2 +-
 security/Kconfig.hardening                  |  3 +
 security/loadpin/loadpin.c                  | 89 +++++++++++++++++------------
 24 files changed, 229 insertions(+), 60 deletions(-)
 create mode 100644 include/linux/ubsan.h

Comments

Linus Torvalds Feb. 21, 2023, 7:16 p.m. UTC | #1
On Fri, Feb 17, 2023 at 11:38 AM Kees Cook <keescook@chromium.org> wrote:
>
> Please pull these hardening updates for v6.3-rc1.

So I've pulled this, but while looking at it, I see commit
5c0f220e1b2d ("Merge branch 'for-linus/hardening' into
for-next/hardening").

And that one-liner shortlog part is literally the whole commit message.

I've said this before, and apparently I need to say this again: if you
cannot be bothered to explain *WHY* a merge exists, then that merge is
buggy garbage by definition.

This really should be a rule that every single developer should take
to heart. I'm not just putting random words together in a random
order.

I repeat: if you cannot explain a merge, then JUST DON'T DO IT.

It's really that simple. There is absolutely *NEVER* an excuse for
merges without explaining why those merges exist.

In this case, I really think that merge should not have existed at
all, and the lack of explanation is because there *IS* no explanation
for it.

But if there was a reason for it, then just state it, dammit, and make
that merge commit look sensible.

Because right now it just looks entirely pointless. And I literally
*detest* pointless merges. They only make the history look worse and
harder to read.

                     Linus
pr-tracker-bot@kernel.org Feb. 21, 2023, 7:29 p.m. UTC | #2
The pull request you sent on Fri, 17 Feb 2023 11:38:18 -0800:

> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/hardening-v6.3-rc1

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

Thank you!
Kees Cook Feb. 21, 2023, 7:49 p.m. UTC | #3
On February 21, 2023 11:16:33 AM PST, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>On Fri, Feb 17, 2023 at 11:38 AM Kees Cook <keescook@chromium.org> wrote:
>>
>> Please pull these hardening updates for v6.3-rc1.
>
>So I've pulled this, but while looking at it, I see commit
>5c0f220e1b2d ("Merge branch 'for-linus/hardening' into
>for-next/hardening").
>
>And that one-liner shortlog part is literally the whole commit message.
>
>I've said this before, and apparently I need to say this again: if you
>cannot be bothered to explain *WHY* a merge exists, then that merge is
>buggy garbage by definition.

Okay, understood. This was a merge of the fixes for v6.2. I'll explain that more clearly in the log from now on. :)

-Kees
Linus Torvalds Feb. 21, 2023, 8:08 p.m. UTC | #4
On Tue, Feb 21, 2023 at 11:49 AM Kees Cook <kees@kernel.org> wrote:
>
> >I've said this before, and apparently I need to say this again: if you
> >cannot be bothered to explain *WHY* a merge exists, then that merge is
> >buggy garbage by definition.
>
> Okay, understood. This was a merge of the fixes for v6.2. I'll explain that more clearly in the log from now on. :)

So I really want people to document their merges, not just so that I
(and others) can see "oh, that's why it exists at all", but also
because I want to make people think about their merges more in
general.

For example, one reason why people do these kinds of merges is because
they are starting to do some new development for the next release, and
that new development then depends on fixes or infrastructure that they
had in another branch (like a "for-linus" branch in case of fixes).

So then they - mindlessly - just do a "git merge that-branch" and the
end result looks very much like what you sent me.

In a slightly better world, they then actually write an explanatory
commit message for that merge, knowing that I ask for them, and the
merge commit message ends up being exactly that kind of slightly odd

  "Now I'm starting a new thing that depends on the fixes
   I already sent upstream, so I'm merging that branch"

Which while certainly better than no explanation at all sounds a bit
odd, doesn't it? Yeah, add a few details on just what you depend on
and why, and it gets much better, but it's all going to be a bit
hand-wavy about future work that you haven't even written yet.

And *that* will them maybe make you then go "Ahh, I'm doing things wrong".

Because the "nice git way" to do that kind of thing is to actually
realize "oh, I'm starting new work that depends on the fixes I already
sent upstram, so I should just make a new topic branch and start at
that point that I needed".

And then - once you've done all the "new work" that depended on that
state, only at *THAT* point do you merge the topic branch.

And look - you have exactly the same commits: you have one (or more)
normal commits that implement the new feature, and you have one merge
commit, but notice how much easier it is to write the explanation for
the merge when you do it *after* the work.

Instead of having to waffle about "future work depends on this feature
that was in another branch, so I'm merging this branch", your merge
commit now makes *sense*. You're not merging some old state in order
to create new features, you are literally just merging the completed
new feature.

So *this* is one reason I want people to really think about, and
explain, their merges. Because it may be that having to explain it
makes you go "Oh, I'm doing this wrong".

Now, in your case, I don't actually think you needed that merge for
any "future new work" at all. I think you just randomly did a merge to
just get the same warning fixes that you had already sent me. So in
this case, it smells like the merge was just entirely superfluous.

Those kinds of superfluous merges can be ok - it's just annoying to
have a development branch that still shows some artifact that you
already fixed elsewhere.

But they still need the explanation. And for that case, I want the
explanation partly to make it clear that you really *thought* about
it, and partly just so that I can see why you did it.

Because we have a very real history where people did mindless daily
back-merges like this "just because" with absolutely no rhyme or
reason, just because they wanted to start each day with the most
recent base, and it really gets very ugly. The development history can
go from a DAG that actually visualizes the different development
streams nicely to a spider-net maze of inexplicable merges very
quickly.

         Linus