On Fri, 2021-04-30 at 07:20 -0700, Patrick Donnelly wrote: > On Fri, Apr 30, 2021 at 6:45 AM Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > > The client can stuff that into the xattr blob when creating a new inode, > > > > and the MDS can scrape it out of that and move the data into the correct > > > > field in the inode. A setxattr on this field would update the new field > > > > too. It's an ugly interface, but shouldn't be too bad to handle and we > > > > have some precedent for this sort of thing. > > > > > > > > The rules for handling the new field in the client would be a bit weird > > > > though. We'll need to allow it to reading the fscrypt_ctx part without > > > > any caps (since that should be static once it's set), but the size > > > > handling needs to be under the same caps as the traditional size field > > > > (Is that Fsx? The rules for this are never quite clear to me.) > > > > > > > > Would it be better to have two different fields here -- fscrypt_auth and > > > > fscrypt_file? Or maybe, fscrypt_static/_dynamic? We don't necessarily > > > > need to keep all of this info together, but it seemed neater that way. > > > > > > I'm not seeing a reason to split the struct. > > > > > > > What caps should this live under? We have different requirements for > > different parts of the struct. > > > > 1) fscrypt context: needs to be always available, especially when an > > inode is initially instantiated, though it should almost always be > > static once it's set. The exception is that an empty directory can grow > > a new context when it's first encrypted, and we'll want other clients to > > pick up on this change when it occurs. > > Do clients need to see this when not reading/writing to the file? > Generally, yes. It's used for regular files when reading/writing, directories for accessing their contents, and for encrypting/decrypting symlink contents. > > 2) "real" size: needs to be under Fwx, I think (though I need to look > > more closely at the truncation path to be sure). > > Frs would need the size as well. > Correct, I was speaking more about what you'd need to cache changes to it. Reads would indeed need Fr or Fs. > > ...and that's not even considering what rules we might want in the > > future for other info we stuff into here. Given that the MDS needs to > > treat this as opaque, what locks/caps should cover this new field? > > I think because the encryption context is used for reads/writes, it > can fall under the same lock domain as the file size. I don't see a > need (yet) for accessing e.g. the encrypted version/blocksize outside > of the Fsx cap. It's good to think about though and I wonder if anyone > else has thoughts on it. > We specifically need this for directories and symlinks during pathwalks too. Eventually we may also want to encrypt certain data for other inode types as well (e.g. block/char devices). That's less critical though. The problem with fetching it after the inode is first instantiated is that we can end up recursing into a separate request while encoding a path. For instance, see this stack trace that Luis reported: https://lore.kernel.org/ceph-devel/53d5bebb28c1e0cd354a336a56bf103d5e3a6344.camel@xxxxxxxxxx/T/#m0f7bbed6280623d761b8b4e70671ed568535d7fa While that implementation stored the context in an xattr, the problem isstill the same if you have to fetch the context in the middle of building a path. The best solution is just to always ensure it's available. -- Jeff Layton <jlayton@xxxxxxxxxx>
