From 911a40265cadf9d3434f594cae5d09eeb009cdf8 Mon Sep 17 00:00:00 2001 From: Christopher Hoffman Date: Wed, 17 Jul 2024 15:25:48 +0000 Subject: [PATCH] client: Add busy case on key removal Fixes: https://tracker.ceph.com/issues/64159 Signed-off-by: Christopher Hoffman --- src/client/Client.cc | 43 +++++++++++++++++++++++++++++++++++++++++-- src/client/Client.h | 1 + src/client/FSCrypt.cc | 42 +++++++++++++++++++++++------------------- src/client/FSCrypt.h | 24 ++++++++++++++++++++++++ src/client/fuse_ll.cc | 1 - 5 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 9494b124e15..a395302fdb7 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -10740,6 +10740,11 @@ int Client::_release_fh(Fh *f) in->unset_deleg(f); if (in->snapid == CEPH_NOSNAP) { + FSCryptKeyHandlerRef kh; + get_keyhandler(in->fscrypt_ctx, kh); + if (kh && kh->di) { + kh->di->del_inode(in->ino); + } if (in->put_open_ref(f->mode)) { _flush(in, new C_Client_FlushComplete(this, in)); check_caps(in, 0); @@ -10796,6 +10801,12 @@ int Client::_open(const InodeRef& in, int flags, mode_t mode, Fh **fhp, int want = ceph_caps_for_mode(cmode); int result = 0; + FSCryptKeyHandlerRef kh; + get_keyhandler(in->fscrypt_ctx, kh); + if (kh && kh->di) { + kh->di->add_inode(in->ino); + } + in->get_open_ref(cmode); // make note of pending open, since it effects _wanted_ caps. int do_sync = true; @@ -10878,6 +10889,11 @@ int Client::_open(const InodeRef& in, int flags, mode_t mode, Fh **fhp, } } } else { + FSCryptKeyHandlerRef kh; + get_keyhandler(in->fscrypt_ctx, kh); + if (kh && kh->di) { + kh->di->del_inode(in->ino); + } in->put_open_ref(cmode); } @@ -15701,6 +15717,12 @@ FSCRYPT /* If the caller passed a value in fhp, do the open */ if(fhp) { + FSCryptKeyHandlerRef kh; + get_keyhandler((*inp)->fscrypt_ctx, kh); + if (kh && kh->di) { + kh->di->add_inode((*inp)->ino); + } + (*inp)->get_open_ref(cmode); *fhp = _create_fh(inp->get(), flags, cmode, perms); } @@ -16196,6 +16218,16 @@ int Client::ll_rmdir(Inode *in, const char *name, const UserPerm& perms) return _rmdir(in, name, perms); } +int Client::get_keyhandler(FSCryptContextRef fscrypt_ctx, FSCryptKeyHandlerRef& kh){ + if (fscrypt_ctx) { + int r = fscrypt->get_key_store().find(fscrypt_ctx->master_key_identifier, kh); + if (kh) + if (kh && kh->di) + return r; + } + return 0; +} + bool Client::is_inode_locked(Inode *to_check) { if (to_check && to_check->fscrypt_ctx) { @@ -16204,6 +16236,9 @@ bool Client::is_inode_locked(Inode *to_check) if (r < 0) { return true; } + if (!kh->present) { + return true; + } } return false; } @@ -18193,7 +18228,6 @@ int Client::add_fscrypt_key(const char *key_data, int key_len, } auto& k = kh->get_key(); - if (kid) { *kid = k->get_identifier(); } @@ -18205,7 +18239,12 @@ int Client::remove_fscrypt_key(fscrypt_remove_key_arg* kid, int user) { auto& key_store = fscrypt->get_key_store(); - return key_store.invalidate(kid, user); + int r = key_store.invalidate(kid, user); + if (kid->removal_status_flags & (FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY == 0)) { + sync_fs(); + } + + return r; } int Client::set_fscrypt_policy_v2(int fd, const struct fscrypt_policy_v2& policy) diff --git a/src/client/Client.h b/src/client/Client.h index c31b107c223..e684e7502f8 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -1997,6 +1997,7 @@ private: int _link(Inode *diri_from, const char* path_from, Inode* diri_to, const char* path_to, const UserPerm& perm, std::string alternate_name); int _unlink(Inode *dir, const char *name, const UserPerm& perm); + int get_keyhandler(FSCryptContextRef fscrypt_ctx, FSCryptKeyHandlerRef& kh); bool is_inode_locked(Inode *to_check); int _rename(Inode *olddir, const char *oname, Inode *ndir, const char *nname, const UserPerm& perm, std::string alternate_name); int _mkdir(const walk_dentry_result& wdr, mode_t mode, const UserPerm& perm, diff --git a/src/client/FSCrypt.cc b/src/client/FSCrypt.cc index 870b1697061..9eae8b93b56 100644 --- a/src/client/FSCrypt.cc +++ b/src/client/FSCrypt.cc @@ -418,32 +418,25 @@ int FSCryptKeyStore::maybe_add_user(std::list* users, int user) int FSCryptKeyStore::maybe_remove_user(struct fscrypt_remove_key_arg* arg, std::list* users, int user) { ldout(cct, 10) << __FILE__ << ":" << __LINE__ << " user=" << user << dendl; - uint32_t status_flags = 0; - int err = 0; - bool removed = false; + uint32_t status_flags = arg->removal_status_flags; + if (!valid_key_spec(arg->key_spec)) { return -EINVAL; } auto it = std::find(users->begin(), users->end(), user); if (it != users->end()) { - ldout(cct, 10) << "maybe_remove_user, user found removing!" << dendl; - removed = true; users->erase(it); - } else { - return -EUSERS; } - ldout(cct, 10) << "maybe_add_user size is now=" << users->size() << dendl; - - if (users->size() != 0 && removed) { + if (users->size() != 0) { //set bits for removed for requested user status_flags |= FSCRYPT_KEY_REMOVAL_STATUS_FLAG_OTHER_USERS; - err = -EBUSY; + arg->removal_status_flags = status_flags; + return -EUSERS; } - arg->removal_status_flags = status_flags; - return err; + return 0; } int FSCryptKeyStore::create(const char *k, int klen, FSCryptKeyHandlerRef& key_handler, int user) @@ -477,7 +470,8 @@ int FSCryptKeyStore::create(const char *k, int klen, FSCryptKeyHandlerRef& key_h if (r == -EEXIST) { return 0; //returns 0 regardless } - + key_handler->present = true; + key_handler->di = new FSCryptDecryptedInodes(); m[id] = key_handler; } @@ -521,15 +515,25 @@ int FSCryptKeyStore::invalidate(struct fscrypt_remove_key_arg* arg, int user) auto& users = kh->get_users(); r = maybe_remove_user(arg, &users, user); - if (r < 0) { - return r; + if (r == -EUSERS) { + r = 0; + goto out; } - kh->reset(++epoch, nullptr); + kh->present = false; - m.erase(id); + //do a final clean up + if (!kh->present && kh->di->get_inodes().empty()) { + kh->reset(++epoch, nullptr); + free(kh->di); + m.erase(id); + } else { + r = 0; + arg->removal_status_flags |= FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY; + } - return 0; +out: + return r; } FSCryptKeyValidator::FSCryptKeyValidator(CephContext *cct, FSCryptKeyHandlerRef& kh, int64_t e) : cct(cct), handler(kh), epoch(e) { diff --git a/src/client/FSCrypt.h b/src/client/FSCrypt.h index a39ea8ea2fc..b209941701c 100644 --- a/src/client/FSCrypt.h +++ b/src/client/FSCrypt.h @@ -294,6 +294,28 @@ public: using FSCryptFDataDencRef = std::shared_ptr; +class FSCryptDecryptedInodes { + std::list decrypted_inodes; + int open_inodes; + +public: + void add_inode(ino_t inode) { + decrypted_inodes.emplace_back(inode); + } + + int del_inode(ino_t inode) { + if (decrypted_inodes.size() == 0) + return 0; + auto it = std::find(decrypted_inodes.begin(), decrypted_inodes.end(), inode); + if (it != decrypted_inodes.end()) + decrypted_inodes.erase(it); + return 0; + } + std::list get_inodes() { + return decrypted_inodes; + } +}; + class FSCryptKeyHandler { ceph::shared_mutex lock = ceph::make_shared_mutex("FSCryptKeyHandler"); int64_t epoch = -1; @@ -308,6 +330,8 @@ public: int64_t get_epoch(); std::list& get_users() { return users; } FSCryptKeyRef& get_key(); + FSCryptDecryptedInodes* di; + bool present = false; }; using FSCryptKeyHandlerRef = std::shared_ptr; diff --git a/src/client/fuse_ll.cc b/src/client/fuse_ll.cc index 5c87306db2f..f59a24137c4 100644 --- a/src/client/fuse_ll.cc +++ b/src/client/fuse_ll.cc @@ -1045,7 +1045,6 @@ static void fuse_ll_ioctl(fuse_req_t req, fuse_ino_t ino, break; } - /* FIXME: handle busy cases */ r = cfuse->client->remove_fscrypt_key(arg, ctx->uid); if (r < 0) { fuse_reply_err(req, -r); -- 2.39.5