Message-ID: <202505312300.95D7D917@keescook>
Date: Sun, 1 Jun 2025 00:42:14 -0700
From: Kees Cook <kees@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Konstantin Ryabitsev <konstantin@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, Eric Biggers <ebiggers@...nel.org>,
	Ingo Saitz <ingo@...nover.ccc.de>,
	kernel test robot <oliver.sang@...el.com>,
	Marco Elver <elver@...gle.com>,
	Nathan Chancellor <nathan@...nel.org>,
	Thiago Jung Bauermann <thiago.bauermann@...aro.org>
Subject: Re: [GIT PULL] hardening fixes for v6.16-rc1

On Sat, May 31, 2025 at 07:35:45PM -0700, Linus Torvalds wrote:
> The rebased history would explain that, but the reason I'm upset about
> it is that I don't even see how that rebasing could possibly happen
> "by mistake".

Here's my reflog, but tl;dr it looks like "b4 trailers" did it. If you
want to skip to that, search for "fast-import" below...


eef1355c269b (HEAD -> for-next/hardening, origin/for-next/hardening, for-next/kspp) HEAD@{0}: checkout: moving from broken/for-next/hardening to for-next/hardening

Getting back to sfr's recommended "known good state".

f8b59a0f90a2 (broken/for-next/hardening) HEAD@{1}: Branch: renamed refs/heads/for-next/hardening to refs/heads/broken/for-next/hardening
f8b59a0f90a2 (broken/for-next/hardening) HEAD@{3}: checkout: moving from broken/for-linus/hardening to for-next/hardening
7ea1ca94c127 (tag: hardening-v6.16-rc1-fix1, origin/for-next/kspp, origin/for-linus/hardening, kees/for-linus/hardening, broken/for-next/kspp, broken/for-linus/hardening) HEAD@{4}: Branch: renamed refs/heads/for-linus/hardening to refs/heads/broken/for-linus/hardening

This is renaming both my for-next and for-linus to "broken/..."

7ea1ca94c127 (tag: hardening-v6.16-rc1-fix1, origin/for-next/kspp, origin/for-linus/hardening, kees/for-linus/hardening, broken/for-next/kspp, broken/for-linus/hardening) HEAD@{6}: am: randstruct: gcc-plugin: Fix attribute addition
2050f9ffa893 HEAD@{7}: commit (amend): overflow: Introduce __DEFINE_FLEX for having no initializer
d316fb5f88c9 HEAD@{8}: checkout: moving from for-next/hardening to for-linus/hardening
f8b59a0f90a2 (broken/for-next/hardening) HEAD@{9}: checkout: moving from fix-rand2 to for-next/hardening

This is amending "overflow: Introduce __DEFINE_FLEX for having no
initializer" and pulling down "randstruct: gcc-plugin: Fix attribute
addition" from lore to get the Tested-by tag -- but it's all going on
top of the broken for-linus/hardening, not having realized it was broken.

32838c389761 (fix-rand2) HEAD@...}: commit (amend): randstruct: gcc-plugin: Fix attribute addition
e4750e3b77b6 HEAD@...}: commit (amend): randstruct: gcc-plugin: Fix attribute addition
3e77d5ec7b16 HEAD@...}: commit (amend): randstruct: gcc-plugin: Fix attribute addition
d8b0249fc36e HEAD@...}: commit: gcc-plugins: randstruct: Fix attribute creation
8dcf80e5097b HEAD@...}: rebase (finish): returning to refs/heads/fix-rand2
8dcf80e5097b HEAD@...}: rebase (pick): overflow: Introduce __DEFINE_FLEX for having no initializer
66989cf66d91 HEAD@...}: rebase (pick): ubsan: integer-overflow: depend on BROKEN to keep this out of CI
8bfebe2e9cc5 HEAD@...}: rebase (pick): wifi: iwlwifi: mld: Work around Clang loop unrolling bug
e0797d3b91de (linux-next/stable, master) HEAD@...}: rebase (start): checkout master
d316fb5f88c9 HEAD@...}: checkout: moving from for-linus/hardening to fix-rand2

This is trying to figure out why merging didn't work, so I restarted on
master and cherry-picked the patches to a separate tree ("fix-rand2")
for testing.

d316fb5f88c9 HEAD@...}: commit (amend): overflow: Introduce __DEFINE_FLEX for having no initializer
8c2917224046 HEAD@...}: commit: overflow: Introduce __DEFINE_FLEX for having no initializer
2e058d249588 HEAD@...}: checkout: moving from for-next/hardening to for-linus/hardening

This is working on and testing "overflow: Introduce __DEFINE_FLEX for having no
initializer".

f8b59a0f90a2 (broken/for-next/hardening) HEAD@...}: reset: moving to f8b59a0f90a2
96e5b773dff6 HEAD@...}: commit: platform/x86: thinkpad_acpi: Handle KCOV __init vs inline mismatches
f8b59a0f90a2 (broken/for-next/hardening) HEAD@...}: am --abort
f8b59a0f90a2 (broken/for-next/hardening) HEAD@...}: reset: moving to f8b59a0f90a2
2e058d249588 HEAD@...}: checkout: moving from for-linus/hardening to for-next/hardening

This is splitting out "platform/x86: thinkpad_acpi: Handle KCOV __init
vs inline mismatches" so I could send it out, and then throwing away the
patch since its going via a separate tree and I need to review that whole
tree separately anyway. (This tree is still the _broken_ for-next tree...)

2e058d249588 HEAD@...}: checkout: moving from for-next/hardening to for-linus/hardening
2e058d249588 HEAD@...}: checkout: moving from test/1 to for-next/hardening
9d230d500b0e HEAD@...}: rebase (skip) (finish): returning to refs/heads/test/1
9d230d500b0e HEAD@...}: rebase (start): checkout master
939b93ecd094 HEAD@...}: checkout: moving from landing/v6.16-rc1-pre/hardening to test/1

I was cleaning up old trees, and went back to look at old commits
(9d230d500b0e) but couldn't figure out why I was having trouble with
merge bases, and tried a rebase but it exploded. I return to the
(broken) for-next.

dbfe626a6fbf (landing/v6.16-rc1-pre/hardening) HEAD@...}: reset: moving to HEAD
dbfe626a6fbf (landing/v6.16-rc1-pre/hardening) HEAD@...}: Branch: renamed refs/heads/dev/hardening to refs/heads/landing/v6.16-rc1-pre/hardening
dbfe626a6fbf (landing/v6.16-rc1-pre/hardening) HEAD@...}: rebase (finish): returning to refs/heads/dev/hardening
dbfe626a6fbf (landing/v6.16-rc1-pre/hardening) HEAD@...}: rebase (pick): ovl: Check for NULL d_inode() in ovl_dentry_upper()
a3ca08cb5fb3 HEAD@...}: rebase (pick): slab: Decouple slab_debug and no_hash_pointers
9d230d500b0e HEAD@...}: rebase (start): checkout master
345b264de969 HEAD@...}: rebase (finish): returning to refs/heads/dev/hardening
345b264de969 HEAD@...}: rebase (pick): ovl: Check for NULL d_inode() in ovl_dentry_upper()
31f107a183e6 HEAD@...}: rebase (pick): drm/amdgpu/atom: Work around vbios NULL offset false positive
3523b5868c43 HEAD@...}: rebase (pick): slab: Decouple slab_debug and no_hash_pointers
8ffd015db85f (tag: v6.15-rc2) HEAD@...}: rebase (start): checkout 8ffd015db85f
541157c72800 HEAD@...}: checkout: moving from for-next/hardening to dev/hardening

Checking if some expected patches have already landed in master while
cleaning up older dev trees and rebasing them forward for potential
revisions during the coming dev cycle. This appears to be sanely based
on master, not a broken tree.

2e058d249588 HEAD@...}: checkout: moving from dev/v6.15-rc4/hardening to for-next/hardening
9d230d500b0e HEAD@...}: rebase (skip) (finish): returning to refs/heads/dev/v6.15-rc4/hardening
9d230d500b0e HEAD@...}: rebase (start): checkout master
b7286d1e8cad HEAD@...}: checkout: moving from dev/v6.16-rc1-pre/-Wunterminated-string-initialization to dev/v6.15-rc4/hardening
62329e859b25 (dev/v6.16-rc1-pre/-Wunterminated-string-initialization) HEAD@...}: checkout: moving from for-next/hardening to dev/v6.16-rc1-pre/-Wunterminated-string-initialization

Trying to figure out why I can't sanely rebase. (Note that 2e058d249588 is on a
broken base tree, repeated above. Below, c102753312e8 is on a sane tree.)

2e058d249588 HEAD@...}: reset: moving to 2e058d249588
bd31653e0d81 HEAD@...}: reset: moving to HEAD
bd31653e0d81 HEAD@...}: fast-import
62329e859b25 (dev/v6.16-rc1-pre/-Wunterminated-string-initialization) HEAD@...}: checkout: moving from test/kern-splat to for-next/hardening
9a7d4e791037 HEAD@...}: reset: moving to 9a7d4e791037
62329e859b25 (dev/v6.16-rc1-pre/-Wunterminated-string-initialization) HEAD@...}: checkout: moving from for-next/hardening to test/kern-splat

This is where 2e058d249588 first appears, and before it is the branch
juggling of one of my scripts to send a single patch out of the middle of
a tree ("kern-splat" was a script to email the top patch, "kern-splat-one"
sends a specific sha from the tree by temporarily making a new branch,
"test/kern-splat", with that sha at the top, using "kern-splat", and
then restoring the tree to the prior state.)

But the "fast-import" is NOT part of that, but rather from "b4
trailers". I checked my reflog against my bash history...

"l" is "git log -1"
"s" is "git show"
"d" is "git diff"
"latr" is "git branch --sort=committerdate"

14029  git commit --amend
14030  s
14031  d
14032  git commit -asm '[DUP]'
14033  l
14034  kern-splat-one 9a7d4e791037
14035  l
14036  b4 trailers -u https://lore.kernel.org/all/CANpmjNPpyJn++DVZmO89ms_HkJ0OvQzkps0GjCFbWkk0F+_8Xg@mail.gmail.com
14037  l
14038  git reset --hard 2e058d249588
14039  l
14040  latr
14041  l dev/v6.16-rc1-pre/-Wunterminated-string-initialization
14042  git reflog
14043  l 62329e859b25
14044  latr
14045  #git checkout 62329e859b25
14046  git branch -D dev/v6.16-rc1-pre/-Wunterminated-string-initialization
14047  git checkout 62329e859b25 -b dev/v6.16-rc1-pre/-Wunterminated-string-initialization
14048  l
14049  latr
14050  git branch -D dev/next-20250516/Wunterminated-string-initialization
14051  git branch -D dev/mld

HEAD@...} below is 14029 above.
HEAD@...} below is 14032 above.

HEAD@...} through HEAD@...} above is 14034 above.

HEAD@...} and HEAD@...} above is 14036 above.

Then I try to throw away (with 14038):

62329e859b25 (dev/v6.16-rc1-pre/-Wunterminated-string-initialization) [DUP]
9a7d4e791037 crypto: Annotate crypto strings with nonstring
b080c44c4d69 kbuild: Re-enable -Wunterminated-string-initialization

and just have "ubsan: integer-overflow: depend on BROKEN to keep this
out of CI" on top, but for some reason it shows in "git log -1" as
2e058d249588 not 8c2bb7d12601.

Now, looking at the tree for 8c2bb7d12601, I see I'm on sane "master"
base. I'll bet "b4 trailers" did something Exciting when rewriting stuff.
More below...

62329e859b25 (dev/v6.16-rc1-pre/-Wunterminated-string-initialization) HEAD@...}: commit: [DUP]
9a7d4e791037 HEAD@...}: commit (amend): crypto: Annotate crypto strings with nonstring
08652ab8b218 HEAD@...}: commit (amend): crypto: Annotate crypto strings with nonstring
7bf10004aed0 HEAD@...}: commit: crypto: Annotate crypto strings with nonstring
b080c44c4d69 HEAD@...}: commit (cherry-pick): kbuild: Re-enable -Wunterminated-string-initialization
8c2bb7d12601 HEAD@...}: rebase (finish): returning to refs/heads/for-next/hardening
8c2bb7d12601 HEAD@...}: rebase (pick): ubsan: integer-overflow: depend on BROKEN to keep this out of CI
b9dbd69a32e3 HEAD@...}: rebase (pick): wifi: iwlwifi: mld: Work around Clang loop unrolling bug
9d230d500b0e HEAD@...}: rebase (start): checkout master

9d230d500b0e and 8c2bb7d12601 are sane trees. I'm pulling forward my
patches to enable -Wunterminated-string-initialization for testing, and
I find a new warning (in crypto) which I make, as well as another that
I'd already fixed before, that I split and leave as "[DUP]", then run
"kern-splat-one" on the crypto patch to get it sent[1].

c102753312e8 HEAD@...}: checkout: moving from dev/v6.16-rc1-pre/-Wunterminated-string-initialization to for-next/hardening
9d230d500b0e HEAD@...}: checkout: moving from for-next/hardening to dev/v6.16-rc1-pre/-Wunterminated-string-initialization
c102753312e8 HEAD@...}: rebase (finish): returning to refs/heads/for-next/hardening
c102753312e8 HEAD@...}: rebase (pick): ubsan: integer-overflow: depend on BROKEN to keep this out of CI
fec8dc564c2f HEAD@...}: rebase (reword): wifi: iwlwifi: mld: Work around Clang loop unrolling bug
368556dd234d HEAD@...}: rebase: fast-forward
f0cd6012c40d (tag: hardening-v6.16-rc1, kees/for-next/kspp, kees/for-next/hardening) HEAD@...}: rebase (start): checkout f0cd6012c40d
70f74ef707fc HEAD@...}: commit (amend): ubsan: integer-overflow: depend on BROKEN to keep this out of CI
eef1355c269b (HEAD -> for-next/hardening, origin/for-next/hardening, for-next/kspp) HEAD@...}: commit (amend): ubsan: integer-overflow: depend on BROKEN to keep this out of CI

c102753312e8 is based on a sane tree, I'm starting to build patches that
I'd like to land in -rc1, based on my existing for-next tree. Which gets
us back to the "known good state", per sfr.


Okay, reproducing the "b4 trailers" steps:

#### start from known good tree
$ git checkout 62329e859b25 -b test/wreckage/before
$ l
62329e859b25 (HEAD -> test/wreckage/before, dev/v6.16-rc1-pre/-Wunterminated-string-initialization) [DUP]
9a7d4e791037 crypto: Annotate crypto strings with nonstring
b080c44c4d69 kbuild: Re-enable -Wunterminated-string-initialization
8c2bb7d12601 ubsan: integer-overflow: depend on BROKEN to keep this out of CI
b9dbd69a32e3 wifi: iwlwifi: mld: Work around Clang loop unrolling bug
9d230d500b0e Merge tag 'driver-core-6.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core
bf373e4c786b Merge tag 'devicetree-for-6.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
8ca154e4910e Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
43db11110730 Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm
12e9b9e5223b Merge tag 'ipe-pr-20250527' of git://git.kernel.org/pub/scm/linux/kernel/git/wufan/ipe
90b83efa6701 (stable/master) Merge tag 'bpf-next-6.16' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next
1b98f357dadd Merge tag 'net-next-6.16' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next
...
### Try to update 8c2bb7d12601 with the Acked-by from the list...
$ b4 trailers -u https://lore.kernel.org/all/CANpmjNPpyJn++DVZmO89ms_HkJ0OvQzkps0GjCFbWkk0F+_8Xg@mail.gmail.com
Finding code-review trailers for 39 commits...
Grabbing thread from lore.kernel.org/all/CANpmjNPpyJn%2B%2BDVZmO89ms_HkJ0OvQzkps0GjCFbWkk0F%2B_8Xg@...l.gmail.com/t.mbox.gz
---
  + Acked-by: Marco Elver <elver@...gle.com>
    https://lore.kernel.org/all/CANpmjNPpyJn%2B%2BDVZmO89ms_HkJ0OvQzkps0GjCFbWkk0F%2B_8Xg@mail.gmail.com
---
Press Enter to apply these trailers or Ctrl-C to abort
  ubsan: integer-overflow: depend on BROKEN to keep this out of CI
    + Acked-by: Marco Elver <elver@...gle.com> (✓ DKIM/google.com)
---
Invoking git-filter-repo to update trailers.
New history written in 3.28 seconds...
Completely finished after 3.76 seconds.
Trailers updated.
$ l
bd31653e0d81 (HEAD -> test/wreckage/before) [DUP]
b68e360e9673 crypto: Annotate crypto strings with nonstring
650370e9729c kbuild: Re-enable -Wunterminated-string-initialization
2e058d249588 ubsan: integer-overflow: depend on BROKEN to keep this out of CI
50d526235542 wifi: iwlwifi: mld: Work around Clang loop unrolling bug
f8b59a0f90a2 (broken/for-next/hardening) Merge tag 'driver-core-6.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core
301559ea27b1 Merge tag 'devicetree-for-6.16' of git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux
ca1f463363e2 Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
...

Welp, that precisely recreated it -- even identical shas! Looking at
the b4 output, I do see a suspicious "39 commits" listed for some reason.

So, I assume the "git-filter-repo" invocation is what mangled it. I will
try to dig into what b4 actually asked it to do in the morning...

-Kees

[1] https://lore.kernel.org/lkml/20250529173113.work.760-kees@kernel.org/

-- 
Kees Cook

Powered by blists - more mailing lists