[git pull] device mapper fixes for 6.8-rc6

[git pull] device mapper fixes for 6.8-rc6

[Mike Snitzer]

Hi Linus,

The following changes since commit 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478:

  Linux 6.8-rc3 (2024-02-04 12:20:36 +0000)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-6.8/dm-fixes-2

for you to fetch changes up to 0e0c50e85a364bb7f1c52c84affe7f9e88c57da7:

  dm-crypt, dm-integrity, dm-verity: bump target version (2024-02-20 13:35:47 -0500)

Please pull, thanks.
Mike

----------------------------------------------------------------
- Stable fixes for 3 DM targets (integrity, verity and crypt) to
  address systemic failure that can occur if user provided pages map
  to the same block.

- Fix DM crypt to not allow modifying data that being encrypted for
  authenticated encryption.

- Fix DM crypt and verity targets to align their respective bvec_iter
  struct members to avoid the need for byte level access (due to
  __packed attribute) that is costly on some arches (like RISC).
-----BEGIN PGP SIGNATURE-----

iQEzBAABCAAdFiEEJfWUX4UqZ4x1O2wixSPxCi2dA1oFAmXY0iwACgkQxSPxCi2d
A1oG3Qf/WE0T5qyBnDZ7irhvJmSLVx4oAwzB0PmMtELZ3Tkyn7BBAxq1Q2I2UT3x
r90d1uy/pz6Y+kZkAPZjYuYLctukEa1swpfFe0Sn01dBrbgGU/p2vi3fkF+ZK6/t
n5EN8S5dkf6rIDmp8R56iP8mP4OEultYjLugxc6ROohFgHZicoqv+Pye9kHp0Y19
HSW2eueag/s2nMa9HKjIEd3+NBgmGb0qMMf3M6CXpRLNi/f/cyHbPzq83+eW3gcg
jl480w5YHk2nOUSqrO8UfIaP4BpD3SEXQxVqIzdkVX4cEBO4yRcBNrQpsT89GsXj
sg5zinkq3g7SThEpQWdpkeZMR/6q/A==
=n0nQ
-----END PGP SIGNATURE-----

----------------------------------------------------------------
Mike Snitzer (1):
      dm-crypt, dm-integrity, dm-verity: bump target version

Mikulas Patocka (5):
      dm-integrity: recheck the integrity tag after a failure
      dm-verity: recheck the hash after a failure
      dm-crypt: don't modify the data when using authenticated encryption
      dm-crypt: recheck the integrity tag after a failure
      dm-verity, dm-crypt: align "struct bvec_iter" correctly

 drivers/md/dm-crypt.c         | 101 ++++++++++++++++++++++++++++++++++--------
 drivers/md/dm-integrity.c     |  95 ++++++++++++++++++++++++++++++++++-----
 drivers/md/dm-verity-target.c |  88 +++++++++++++++++++++++++++++++++---
 drivers/md/dm-verity.h        |  10 ++++-
 4 files changed, 256 insertions(+), 38 deletions(-)

Re: [git pull] device mapper fixes for 6.8-rc6

[Linus Torvalds]

On Fri, 23 Feb 2024 at 09:17, Mike Snitzer <snitzer@kernel.org> wrote:
>
> - Fix DM crypt and verity targets to align their respective bvec_iter
>   struct members to avoid the need for byte level access (due to
>   __packed attribute) that is costly on some arches (like RISC).

Ugh. This is due to commit 19416123ab3e ("block: define 'struct
bvec_iter' as packed"), and the point of *that* commit was that it
doesn't hurt to mark it packed.

That was clearly not true.

And honestly, "__packed" really is wrong here.  Nobody ever wanted it
to be completely unaligned.

I think we might be better off marking it as being 4-byte aligned.
That would mean that instead of __packed, it is done as

   __packed __aligned(4)

because "__aligned" on its own only increases alignment (so without
the __packed it would stay 8-byte aligned).

Then the only part of that structure that might be unaligned is
"sector_t", and that would only matter on 64-bit architectures.

 And very few architectures are both 64-bit _and_ so broken as to not
do unaligned loads well. And even if such broken architectures exist,
at least they can do the 8-byte load as two 4-byte ones rather than
doing it as byte loads..

Anyway, I've pulled this, but I really think this should have been
fixed in bvec.h instead.

Jens/Christoph/whoever feels they own bvec.h?

                Linus

Re: [git pull] device mapper fixes for 6.8-rc6

[Christoph Hellwig]

On Fri, Feb 23, 2024 at 09:42:21AM -0800, Linus Torvalds wrote:
> And honestly, "__packed" really is wrong here.  Nobody ever wanted it
> to be completely unaligned.

I'll let Ming speak, but I think the idea was to remove the padding
at the end of the structure when embedded into the bio.

> 
> I think we might be better off marking it as being 4-byte aligned.
> That would mean that instead of __packed, it is done as
> 
>    __packed __aligned(4)
>
> 
> because "__aligned" on its own only increases alignment (so without
> the __packed it would stay 8-byte aligned).
> 
> Then the only part of that structure that might be unaligned is
> "sector_t", and that would only matter on 64-bit architectures.

Does __aligned also work on struct members?  If so we could add a
__aligned(8) to bi_sector an get exactly what we want..

Re: [git pull] device mapper fixes for 6.8-rc6

[Linus Torvalds]

On Fri, 23 Feb 2024 at 09:46, Christoph Hellwig <hch@lst.de> wrote:
>
> I'll let Ming speak, but I think the idea was to remove the padding
> at the end of the structure when embedded into the bio.

It's not horribly obvious if the beginning is aligned there either.

> Does __aligned also work on struct members?  If so we could add a
> __aligned(8) to bi_sector an get exactly what we want..

Hmm. I'm not sure that works. I think sizeof may always end up being
aligned to alignof (because otherwise arrays cannot work)

And looking at

    struct bio_integrity_payload {

there's odd padding both before _and_ after the struct bvec_iter due
to having three 16-bit fields in between.

So right now I think that packing ends up actually horrid. I don't see
any *reason* for that odd setup, but right now it might have actually
end up being 2-byte aligned with a two-byte padding hole at the end.

             Linus

Re: [git pull] device mapper fixes for 6.8-rc6

[pr-tracker-bot]

The pull request you sent on Fri, 23 Feb 2024 12:17:37 -0500:

> git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-6.8/dm-fixes-2

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

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html