Hello, While getting the fstests stuff nailed down to deal with btrfs I ran into failures with generic/595, specifically the multi-threaded part. In one thread we have a loop adding and removing the master key. In the other thread we have us trying to echo a character into a flie in the encrypted side, and if it succeeds we echo a character into a temporary file, and then after the runtime has elapsed we compare these two files to make sure they match. The problem with this is that btrfs derives the per-extent key from the master key at writeout time. Everybody else has their content key derived at flie open time, so they don't need the master key to be around once the file is opened, so any writes that occur while that file is held open are allowed to happen. Sweet Tea had some changes around soft unloading the master key to handle this case. Basically we allow the master key to stick around by anybody who may need it who is currently open, and then any new users get denied. Once all the outstanding open files are closed the master key is unloaded. This keeps the semantics of what happens for everybody else. What is currently happening with my version of the patchset, which didn't bring in those patches, is that you get an ENOKEY at writeout time if you remove the key. The fstest fails because even tho we let you write to the file sometimes, it doesn't necessarily mean it'll make it to disk. If we want to keep the semantics of "when userspace tells us to throw away the master key, we absolutely throw the master key away" then I can just make adjustments to the fstests test and call what I have good enough. If we want to have the semantics of "when userspace tells us to throw away the master key we'll make it unavailable for any new users, but existing open files operate as normal" then I can pull in Sweet Tea's soft removal patches and call it good enough. There's a third option that is a bit of a middle ground with varying degrees of raciness. We could sync the file system before we do the removal just to narrow the window where we could successfully write to a file but get an ENOKEY at writeout time. We could freeze the filesystem to make sure it's sync'ed and allow any current writers to complete, this would be a stronger version of the first option, again just narrows the window. Neither of these cases help if the file is being held open. If we wanted to fully deal with the file being held open case we could set a flag, sync, then remove the key. Then we add a new fscrypt_prep_write() hook that filesystems could optionally use, obviously just btrfs for now, that we'd stick in the write path that would check for this flag or if the master key had been removed so we can deny dirtying when the key is removed. At this point I don't have strong opinions, it's easier for me to just leave it like it is and change fstests. Anything else is a change in the semantics of how the master key is handled, and that's not really a decision I feel comfortable making for everybody. Once we nail this detail down I can send the updated version of all the patches and we can start talking about inclusion. Thanks, Josef
|