[LU-3671] why are permission changes synchronous? Created: 30/Jul/13 Updated: 06/Nov/18 Resolved: 04/Sep/13 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | Lustre 2.4.0 |
| Fix Version/s: | Lustre 2.5.0 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Ned Bass | Assignee: | Lai Siyao |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | zfs | ||
| Issue Links: |
|
||||||||||||||||||||
| Severity: | 3 | ||||||||||||||||||||
| Rank (Obsolete): | 9459 | ||||||||||||||||||||
| Description |
|
mdd_acl_set() sets the ACL xattr synchronously if /proc/fs/lustre/mdd/lustre-MDT0000/sync_permission is set (it is set by default). This can have a large performance impact for operations like cp -a or tar --xattr. Should this be the default behavior? How does this relate to other permissions-related changed like chown, chmod, etc.? It would be good to explain the security implications of this setting in code comments and in the Lustre manual. Some background information: http://wiki.lustre.org/index.php/Architecture_-_Version_Based_Recovery#Permissions LLNL-bug-ID: TOSS-2207 |
| Comments |
| Comment by Christopher Morrone [ 30/Jul/13 ] |
|
This (and/or |
| Comment by Ned Bass [ 30/Jul/13 ] |
|
This setting does apply to chmod, chown, etc. In a VM environment with ZFS backend, chmod or chown of 60 files takes about 15s with sync_permission=1, versus 100ms with sync_permission=0. |
| Comment by Peter Jones [ 30/Jul/13 ] |
|
Lai This seems related to Peter |
| Comment by Ned Bass [ 30/Jul/13 ] |
|
To clarify our immediate priority here, how risky would it be to turn off sync_permission? We're feeling significant pain due to this issue on our production filesystems, so we'd like to take prompt action if its reasonably safe to do so. My impression is that we're trying to prevent something like an open() getting incorrectly replayed before a chmod() during recovery, allowing a client to circumvent permissions. This seems fairly low risk, but I'm not sure I understand this issue completely. |
| Comment by Mikhail Pershin [ 31/Jul/13 ] |
|
IIRC, there is no harm or bug related to sync_permission, that is just an attempt to make any permission changes persistent and be not lost due to recovery. The sync_permission guarantees that changes are persistently stored when reply is got. With sync_permission == 0 we might have lost changes due to recovery if client node isn't alive, e.g. powered off right after command was executed and return success but changes weren't committed yet on server. |
| Comment by Ned Bass [ 31/Jul/13 ] |
|
Mikhail, thanks, but in that case I don't understand the optimizations discussed by Robert and Andreas in bz15390. They claim the sync is only needed on directories, and only when the permissions are being reduced. |
| Comment by Lai Siyao [ 07/Aug/13 ] |
|
I made a patch according to the optimization proposal mentioned above: http://review.whamcloud.com/7257 |
| Comment by Jodi Levi (Inactive) [ 03/Sep/13 ] |
|
Did the patch landed fix this issue? |
| Comment by Ned Bass [ 03/Sep/13 ] |
|
The patch addresses the performance issue, but not the lack of documentation. I would like to see a step-by-step example scenario in which an asynchronous permission update leads to an inconsistent or insecure state. The example should explain why the problem is only relevant for directories. In other words, explain in precise terms the risks of disabling sync_permission. |
| Comment by Andreas Dilger [ 03/Sep/13 ] |
|
Ned, # umask is 022, so dir1 is created with rwxr-xr-x permission client1$ mkdir /lustre/dir1 client2$ mkdir /lustre/dir2 client1$ chmod 700 /lustre/dir1 client3$ touch /lustre/dir1/file3 client4$ touch /lustre/dir1/file4 : : If client2 fails at the same time as the MDS (right after dir1 is created) and does not participate during MDS recovery, old Lustre recovery would prevent client[34] from creating file[34] because there would be a gap in the MDS transaction sequence, even though there is no dependency between these files and dir2. Similarly, if client1 failed, then file2 and file3 would not be able to recover, even if the dir1 creation was committed on the MDT before if crashed. With VBR, the replay for file3 and file4 would be dependent on the version of dir1 (transaction number in which dir1 was created/last modified), and not on each other. That would allow the files to be recreated from any running client, and only files created by the failing node would be lost. The sync_permission flag is concerned with avoiding the case where client1 fails after creating dir1 and running chmod, but the MDS only committed the mkdir and not the chmod before it fails. That would potentially allow the file creations to be replayed in a directory that does not have the correct permissions. Mike, thinking about this further, is the version of dir1 changed by the chmod so that the later file creates are dependent upon the new version of dir1 and not the old one? That would also prevent the later files to be created without any sync at all, though in most cases where permission changes are not being done this would increase the number of unreplayable RPCs in case of MDS failure. Could you please further clarify what specific problem the sync_permission behaviour is avoiding? |
| Comment by Robert Read (Inactive) [ 03/Sep/13 ] |
|
Suggestion: That example (once the details have been confirmed) would make an excellent addition to the manual. |
| Comment by Ned Bass [ 04/Sep/13 ] |
|
Andreas, thanks for the explanation, but the pieces still aren't quite fitting together for me. In your example, dir1 perms change from 755 to 700, so non-owners lack write permission before and after the chmod. I'm not sure if that's what you intended. Given the optimization recently landed, we're only interested in the case where permissions are reduced. In that case, I imagine the spirit of the example is that touch would normally succeed before the chmod, but fail after. But if the chmod isn't replayed in recovery, client3 cannot distinguish that from the case where client1 never ran chmod (because it crashed), and therefore it is perfectly consistent to allow the touch to succeed. The directory would still have the old permissions which permitted file creation. |
| Comment by Ned Bass [ 04/Sep/13 ] |
|
I think I understand the point of the example now. The touch is predicated on the knowledge that chmod succeeded and the directory has secure permissions. If the chmod is lost, the directory contents become exposed to unauthorized users. I was assuming in my last comment that client3 couldn't see the results of the chmod until it was committed to disk, but now I suspect that it could. Do I have that right? |
| Comment by Mikhail Pershin [ 04/Sep/13 ] |
|
Andreas, yes, chmod changed version of directory, so depended replays will be denied. But nothing prevents just regular creations in that directory after recovery, it will stay without permission for a while. So sync_permission flag is not just about recovery but more like policy when server guarantee all such changes to be done for sure, without sync_permission all works as before but permission might be lost, note this problem exists in all Lustre versions and is not about VBR. All further optimization are just attempts to find some balance between security and performance. |
| Comment by Andreas Dilger [ 04/Sep/13 ] |
|
Ned, your second assessment is correct, and I should have made that more clear in my comment. The danger is that the user thinks the chmod (or chown by root) succeeded, but it was lost during recovery, and this exposes files in the directory that shouldn't be visible to other users. Mike is correct that if file3/4 are created before MDS recovery (while the chmod/chown is still in the MDS cache) then they will be refused during replay because the dir1 permission change was lost when client1 failed and the creates will depend on the now-missing dir1 version. However, if file3/4 are created after MDS recovery (with chmod/chown lost) they will succeed. I guess sync_permissions is over & above what POSIX requires even of a local filesystem, since a local filesystem would lose the chmod after a crash and the user might forget to re-do it, but at least there was a visible interruption to the user when the local node crashed. With a distributed filesystem, the failing node might be out of sight of the user. |
| Comment by Andreas Dilger [ 04/Sep/13 ] |
|
I filed LUDOC-180 to track the documentation for this /proc tunable, and this one can be closed since the patch to avoid sync operations for regular files and non-permission setattrs has landed for 2.5.0. It could potentially also be cherry-picked for 2.4.x and 2.1.x. |