Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix block cloning between unencrypted and encrypted datasets #15465

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

mmatuska
Copy link
Contributor

@mmatuska mmatuska commented Oct 30, 2023

Motivation and Context

When copying data from an unencrypted into an encrypted dataset and vice versa, block cloning should be not used. The current code checked only if the source was encrypted by reading the BP so when zfs_clone_range() was used to copy from unencrypted to encrypted the data remained unencrypted and caused a panic on read.

Fixes kernel panic in #15464

Description

Early check for zfs_clone_range() if we are copying from an unencrypted into an encrypted dataset or vice versa.

How Has This Been Tested?

Linux and FreeBSD

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

module/zfs/zfs_vnops.c Outdated Show resolved Hide resolved
Block cloning from an encrypted dataset into an unencrypted dataset
and vice versa is not possible. The current code did allow cloning
unencrypted files into an encrypted dataset causing a panic when
these were accessed. Block cloning between encrypted and encrypted
is currently supported on the same filesystem only.

Fixes openzfs#15464

Signed-off-by:  Martin Matuska <[email protected]>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Oct 30, 2023
@behlendorf
Copy link
Contributor

cc: @pjd @robn

Copy link
Member

@robn robn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find, and doing the check early is the best way.

We don't actually need anything from the block pointer to do the encryption checks, so the encrypted+same-filesystem check could also be lifted like this. But I wouldn't do that here; it all has to be revisited for encrypted cross-dataset anyway, and this is a nice narrow patch.

Copy link
Contributor

@oromenahar oromenahar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me. All the edge cases... thanks for the patch.
We will check again for enrypted datasets and if they are not the same. So we do not need to pay attention here.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Oct 30, 2023
@behlendorf behlendorf merged commit 763ca47 into openzfs:master Oct 31, 2023
19 checks passed
mmatuska added a commit to mmatuska/zfs that referenced this pull request Nov 3, 2023
Block cloning from an encrypted dataset into an unencrypted dataset
and vice versa is not possible. The current code did allow cloning
unencrypted files into an encrypted dataset causing a panic when
these were accessed. Block cloning between encrypted and encrypted
is currently supported on the same filesystem only.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Rob N <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Martin Matuska <[email protected]>
Closes openzfs#15464
Closes openzfs#15465
behlendorf pushed a commit that referenced this pull request Nov 6, 2023
Block cloning from an encrypted dataset into an unencrypted dataset
and vice versa is not possible. The current code did allow cloning
unencrypted files into an encrypted dataset causing a panic when
these were accessed. Block cloning between encrypted and encrypted
is currently supported on the same filesystem only.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Rob N <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Martin Matuska <[email protected]>
Closes #15464
Closes #15465
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
Block cloning from an encrypted dataset into an unencrypted dataset
and vice versa is not possible. The current code did allow cloning
unencrypted files into an encrypted dataset causing a panic when
these were accessed. Block cloning between encrypted and encrypted
is currently supported on the same filesystem only.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Kay Pedersen <[email protected]>
Reviewed-by: Rob N <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Martin Matuska <[email protected]>
Closes openzfs#15464
Closes openzfs#15465
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants