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

fsync set policy ioctls #359

Merged
merged 1 commit into from
Aug 23, 2022
Merged

Conversation

irq0
Copy link
Contributor

@irq0 irq0 commented Aug 12, 2022

Split policyIoctl into setPolicyIoctl and getPolicyIoctl. Add a
os.Sync() call to setPolicyIoctl.

Policy ioctls are not necessary durable on return. For example, on
ext4 (ref: fs/ext4/crypto.c: ext4_set_context) they are not. This may
lead to a filesystem containing fscrypt metadata (in .fscrypt), but
without the policy applied on an encrypted directory.

Example:
Snapshotting a mounted ext4 filesystem on Ceph RBD right after
setting the policy. While subject to timing, with high probability the
snapshot will not have the policy set. Calling fsync fixes this.

Signed-off-by: Marcel Lauhoff marcel.lauhoff@suse.com

@google-cla
Copy link

google-cla bot commented Aug 12, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Aug 12, 2022
metadata/policy.go Outdated Show resolved Hide resolved
Split policyIoctl into setPolicyIoctl and getPolicyIoctl. Add a
os.Sync() call to setPolicyIoctl.

Policy ioctls are not necessary durable on return. For example, on
ext4 (ref: fs/ext4/crypto.c: ext4_set_context) they are not. This may
lead to a filesystem containing fscrypt metadata (in .fscrypt), but
without the policy applied on an encrypted directory.

Example:
Snapshotting a mounted ext4 filesystem on Ceph RBD right after
setting the policy. While subject to timing, with high probability the
snapshot will not have the policy set. Calling fsync fixes this.

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Aug 17, 2022
Remove if google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
@ebiggers
Copy link
Collaborator

The failure in "Run command-line interface tests" is unrelated and will be fixed by #362.

So it looks like you just need to sign the CLA. (Or make sure that you've done everything for the CLA to be recognized, if your company already has it on file.)

@irq0
Copy link
Contributor Author

irq0 commented Aug 19, 2022

So it looks like you just need to sign the CLA. (Or make sure that you've done everything for the CLA to be recognized, if your company already has it on file.)

There is already a company CLA, I'm waiting to get added.

irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Aug 19, 2022
Remove if google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
@irq0
Copy link
Contributor Author

irq0 commented Aug 23, 2022

CLA is now signed

@ebiggers ebiggers merged commit 75cf580 into google:master Aug 23, 2022
@ebiggers
Copy link
Collaborator

Merged, thanks!

irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Aug 24, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Aug 24, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Aug 30, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Sep 6, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Sep 20, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Sep 26, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Oct 13, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
irq0 pushed a commit to irq0/ceph-csi that referenced this pull request Oct 14, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
Madhu-1 pushed a commit to irq0/ceph-csi that referenced this pull request Oct 17, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
mergify bot pushed a commit to ceph/ceph-csi that referenced this pull request Oct 17, 2022
Revert once our google/fscrypt dependency is upgraded to a version
that includes google/fscrypt#359 gets accepted

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants