On 2014-05-23 16:24:05, Yann Droneaud wrote: > Hi, > > While trying to investigate why using ecryptfs is painfully sluggish, When is it painfully sluggish for you? The two biggest pain points, off of the top of my head, are: 1) Extending a file to a much larger size is slow because pages of zeros are encrypted in the process 2) Utils such as find and ls are slow when dealing with large numbers of files because, during lookups, the eCryptfs metadata has to be read from the front of each lower file in order to fill the eCryptfs inode > I've find a piece of code that, I believed at first, wouldn't scale. > > After adding some printk() there, I've found that it's not at all > called in my use case. > So I wonder: what's making ecryptfs_privileged_open()[1] submit work > to ecryptfs-kthread thread running ecryptfs_threadfn()[2] ? > > [1] > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/ecryptfs/kthread.c#n155 > > [2] > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/ecryptfs/kthread.c#n56 This change (746f1e55) happened before my time of maintaining eCryptfs, but we can piece together the reasoning... Assume that an eCryptfs mount, /upper, is mounted on /lower and keep in mind that there is only ever one lower file opened for all open eCryptfs files that correspond to the lower inode. So, if two processes have open file descriptors for /upper/foo, then /lower/foo is only opened once by the eCryptfs and a pointer to the lower file struct is attached to the eCryptfs inode's private data. Since there's only ever one lower file opened for a given lower inode, it must be opened as O_RDWR to satisfy all future open()s of the eCryptfs file, as one of those future eCryptfs open()s may be O_RDWR. The last paragraph of the 746f1e55 commit message, indicates that the kthread was intended to work around DAC issues when opening the lower file. That may have been the case back then, but it isn't the case now. Today, dentry_open() succeeds even when the lower inode isn't writeable by the current process. Following the dentry_open() code path, I see that security_file_open() is called from do_dentry_open(). That means that even though the kthread is no longer needed from a DAC standpoint, it is still needed from the LSM standpoint. I'll use AppArmor for a simple demonstration: 1) Create a profile, "test", that grants access to all files but then revokes write permissions to /lower/foo $ echo "profile test { file, deny file /lower/foo w, }" | \ sudo apparmor_parser -rq 2) Create an eCryptfs the file - eCryptfs will create the lower file $ touch /upper/foo 3) Open the eCryptfs file with O_RDONLY - It will succeed $ cat /upper/foo 4) Open the eCryptfs file with O_RDONLY, while confined - It should succeed due to falling back to ecryptfs-kthread $ aa-exec -p test -- cat /upper/foo 5) Remove the ecryptfs-kthread code, recompile the eCryptfs module, load the new module and set up the eCryptfs mount again 6) Repeat step #4 - It will fail this time since there's no ecryptfs-kthread $ aa-exec -p test -- cat /upper/foo cat: /upper/foo: Permission denied 7) Note the eCryptfs error messages in the syslog [ 3395.362511] Error opening lower file for lower_dentry [0xffff88007461c288] and lower_mnt [0xffff88007a105420]; rc = [-13] [ 3395.375967] ecryptfs_open: Error attempting to initialize the lower file for the dentry with name [foo]; rc = [-13] > BTW, I have a patch that remove this kernel thread as I think > credentials can be given to dentry_open(), so instead of running an > helper kthread, some privileged credentials can be given to > dentry_open() in the case ecryptfs_privileged_open() really need to > open a file read/write. But as I'm not able to verify it's working as > expected, I'm holding such trival patch. This made me think of a patch set from years ago that, unfortunately, went unreviewed: https://lkml.org/lkml/2011/4/29/104 I think Roberto probably had the right idea. By default, all lower inodes are opened using a cred that was set up at eCryptfs mount time. Optionally, an eCryptfs mount option can be given to specify the LSM security context that should be used when opening the lower inodes. Would you have any interest in reviving that patch set? If so, note that although I used AppArmor in my example above, you'd need to use SELinux or SMACK to test the ecryptfs_security_ctx mount option since AppArmor doesn't have support for set_security_override_from_ctx(). Tyler
Attachment:
signature.asc
Description: Digital signature