[LU-15608] DIO broken for encrypted files

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.15.0
• Fix Version/s: Lustre 2.15.0
• Type: Bug
• Priority: Blocker
• Reporter: Sebastien Buisson
• Assignee: Sebastien Buisson
• Resolution: Fixed
• Labels: encryption
• Severity: 3

Description

In case of direct IO, the page index is not calculated correctly when doing encryption and decryption. With Direct IO, because we do not have proper page cache pages, we need to retrieve by ourselves the page mapping and the page index of the page to be encrypted/decrypted. For this we use (struct brw_page*)->off. Unfortunately, this offset is not relative to the whole file, but it is just an offset within the object. At the end of the road, it means we do not use consistent indexes between the buffered IO and direct IO cases. And more specifically, the indexes used by the direct IO case can repeat (if we access pages at the same offset but in different stripes), which is bad.

Comments

[Andreas Dilger]

Does it make sense to properly initialize the page->index value on the DIO pages before they are submitted, so that this can be used by the fscrypt code?

[Sebastien Buisson]

As these pages are not page cache pages, I would be afraid to break something by changing their ->index value.

[Gerrit Updater]

"Sebastien Buisson <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/46664
Subject: LU-15608 sec: use correct page index for DIO on enc file
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 0b27b8934ab72936baee54b1973e9ea9611b1507

[Gerrit Updater]

"Sebastien Buisson <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/46673
Subject: LU-15608 debug: test correctness of DIO for encrypted files
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 4d7427cb21ff967830c9cb749985af5916c06ce4

[Gerrit Updater]

"Sebastien Buisson <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/46674
Subject: LU-15608 debug: test correctness of DIO for encrypted files
Project: fs/lustre-release
Branch: b2_14
Current Patch Set: 1
Commit: 04bba737098b551a2faf2cda01085700150f444f

[Andreas Dilger]

I was wondering whether there needs to be a way to allow turning this fix off, if there were files previously only read/written by O_DIRECT and an old version of the code? It seems somewhat unlikely, but possible, and would appear to userspace as their upgrade resulted in all files becoming corrupt...

I don't think there are a lot of users running 2.14, fewer of those running fscrypt, and even fewer of those that would use O_DIRECT, but something to keep in mind in the future if there is such a problem.

[Sebastien Buisson]

I would rather publish a note explaining that encrypted files created in 2.14 with O_DIRECT are likely to be corrupted when upgrading to 2.15, so they need to be recreated without O_DIRECT ('read with O_DIRECT' > 'write without O_DIRECT') before upgrading to 2.15.

As you say, ending up in this situation is a pretty unlikely combination of factors, and I am not a big fan of adding an on/off switch for a fix

[Gerrit Updater]

"Oleg Drokin <[e-mail]>" merged in patch https://review.whamcloud.com/46664/
Subject: LU-15608 sec: fix DIO for encrypted files
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 966ca46e4aa2eb39c70e49648ffe6fcaaf475536

[Gerrit Updater]

"Sebastien Buisson <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/46855
Subject: LU-15608 sec: fix DIO for encrypted files
Project: fs/lustre-release
Branch: b2_14
Current Patch Set: 1
Commit: faa620a653421e08d17614bea28c093356ad49b2

[Peter Jones]

Landed for 2.15