Details
-
Bug
-
Resolution: Fixed
-
Critical
-
Lustre 2.4.0
-
3
-
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
https://projectlava.xyratex.com/show_bug.cgi?id=15390
http://git.whamcloud.com/gitweb?p=lustre.git;a=commit;h=0509770778396825ba1d7f1b42be5993aac1d6f3
LLNL-bug-ID: TOSS-2207
Attachments
Issue Links
Activity
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?
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.
Suggestion: That example (once the details have been confirmed) would make an excellent addition to the manual.
Ned,
the specific problem that is being avoided here relates to Version Based Recovery (VBR), is a mechanism to allow some clients to recover from an MDS failure even in the (not so uncommon) case that one or more other active clients do not reconnect MDS and do their own recovery. Basic version-based recovery in the following situation:
# 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?
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.
I made a patch according to the optimization proposal mentioned above: http://review.whamcloud.com/7257
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.
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.
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.
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.