From 928bb69f3943c68f2c53bcd97a6625bfd4f7072a Mon Sep 17 00:00:00 2001 From: Xiubo Li Date: Fri, 9 Sep 2022 12:17:06 +0800 Subject: [PATCH] client: always set the caller_uid/gid to -1 Since the setattr will check the cephx mds auth access before buffering the changes, so it makes no sense any more to let the cap update to check the access in MDS again. Fixes: https://tracker.ceph.com/issues/57154 Signed-off-by: Xiubo Li (cherry picked from commit f474203aee2ff183d6812a62f43d85b3f25eae0d) Conflicts: src/client/Client.cc: missed dependency commit a8d0158d0df ("Client/Inode: wait_for_caps fixups") --- src/client/Client.cc | 80 ++++++++++---------------------------------- src/client/Inode.h | 3 -- 2 files changed, 18 insertions(+), 65 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 8696bbc5880db..7c23c52bdc764 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -3816,8 +3816,17 @@ void Client::send_cap(Inode *in, MetaSession *session, Cap *cap, flush, cap->mseq, cap_epoch_barrier); - m->caller_uid = in->cap_dirtier_uid; - m->caller_gid = in->cap_dirtier_gid; + /* + * Since the setattr will check the cephx mds auth access before + * buffering the changes, so it makes no sense any more to let + * the cap update to check the access in MDS again. + * + * For new clients with old MDSs that doesn't support + * CEPHFS_FEATURE_MDS_AUTH_CAPS_CHECK we will force the session + * to be readonly if root_squash is enabled as a workaround. + */ + m->caller_uid = -1; + m->caller_gid = -1; m->head.issue_seq = cap->issue_seq; m->set_tid(flush_tid); @@ -4092,8 +4101,6 @@ void Client::queue_cap_snap(Inode *in, SnapContext& old_snapc) capsnap.btime = in->btime; capsnap.xattrs = in->xattrs; capsnap.xattr_version = in->xattr_version; - capsnap.cap_dirtier_uid = in->cap_dirtier_uid; - capsnap.cap_dirtier_gid = in->cap_dirtier_gid; if (used & CEPH_CAP_FILE_WR) { ldout(cct, 10) << __func__ << " WR used on " << *in << dendl; @@ -4117,12 +4124,6 @@ void Client::finish_cap_snap(Inode *in, CapSnap &capsnap, int used) capsnap.change_attr = in->change_attr; capsnap.dirty |= in->caps_dirty(); - /* Only reset it if it wasn't set before */ - if (capsnap.cap_dirtier_uid == -1) { - capsnap.cap_dirtier_uid = in->cap_dirtier_uid; - capsnap.cap_dirtier_gid = in->cap_dirtier_gid; - } - if (capsnap.dirty & CEPH_CAP_FILE_WR) { capsnap.inline_data = in->inline_data; capsnap.inline_version = in->inline_version; @@ -4146,8 +4147,13 @@ void Client::send_flush_snap(Inode *in, MetaSession *session, auto m = make_message(CEPH_CAP_OP_FLUSHSNAP, in->ino, in->snaprealm->ino, 0, in->auth_cap->mseq, cap_epoch_barrier); - m->caller_uid = capsnap.cap_dirtier_uid; - m->caller_gid = capsnap.cap_dirtier_gid; + /* + * Since the setattr will check the cephx mds auth access before + * buffering the changes, so it makes no sense any more to let + * the cap update to check the access in MDS again. + */ + m->caller_uid = -1; + m->caller_gid = -1; m->set_client_tid(capsnap.flush_tid); m->head.snap_follows = follows; @@ -5569,11 +5575,6 @@ void Client::handle_cap_flush_ack(MetaSession *session, Inode *in, Cap *cap, con sync_cond.notify_all(); } - if (!dirty) { - in->cap_dirtier_uid = -1; - in->cap_dirtier_gid = -1; - } - if (!cleaned) { ldout(cct, 10) << " tid " << m->get_client_tid() << " != any cap bit tids" << dendl; } else { @@ -8022,27 +8023,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, memset(&args, 0, sizeof(args)); - // make the change locally? - if ((in->cap_dirtier_uid >= 0 && perms.uid() != in->cap_dirtier_uid) || - (in->cap_dirtier_gid >= 0 && perms.gid() != in->cap_dirtier_gid)) { - ldout(cct, 10) << __func__ << " caller " << perms.uid() << ":" << perms.gid() - << " != cap dirtier " << in->cap_dirtier_uid << ":" - << in->cap_dirtier_gid << ", forcing sync setattr" - << dendl; - /* - * This works because we implicitly flush the caps as part of the - * request, so the cap update check will happen with the writeback - * cap context, and then the setattr check will happen with the - * caller's context. - * - * In reality this pattern is likely pretty rare (different users - * setattr'ing the same file). If that turns out not to be the - * case later, we can build a more complex pipelined cap writeback - * infrastructure... - */ - mask |= CEPH_SETATTR_CTIME; - } - bool do_sync = true; int res; { @@ -8063,8 +8043,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!mask) { // caller just needs us to bump the ctime in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); if (issued & CEPH_CAP_AUTH_EXCL) in->mark_caps_dirty(CEPH_CAP_AUTH_EXCL); else if (issued & CEPH_CAP_FILE_EXCL) @@ -8084,8 +8062,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!do_sync && in->caps_issued_mask(CEPH_CAP_AUTH_EXCL)) { in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->uid = stx->stx_uid; in->mark_caps_dirty(CEPH_CAP_AUTH_EXCL); mask &= ~CEPH_SETATTR_UID; @@ -8104,8 +8080,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!do_sync && in->caps_issued_mask(CEPH_CAP_AUTH_EXCL)) { in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->gid = stx->stx_gid; in->mark_caps_dirty(CEPH_CAP_AUTH_EXCL); mask &= ~CEPH_SETATTR_GID; @@ -8124,8 +8098,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!do_sync && in->caps_issued_mask(CEPH_CAP_AUTH_EXCL)) { in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->mode = (in->mode & ~07777) | (stx->stx_mode & 07777); in->mark_caps_dirty(CEPH_CAP_AUTH_EXCL); mask &= ~CEPH_SETATTR_MODE; @@ -8156,8 +8128,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!do_sync && in->caps_issued_mask(CEPH_CAP_AUTH_EXCL)) { in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->btime = utime_t(stx->stx_btime); in->mark_caps_dirty(CEPH_CAP_AUTH_EXCL); mask &= ~CEPH_SETATTR_BTIME; @@ -8176,8 +8146,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!do_sync && in->caps_issued_mask(CEPH_CAP_AUTH_EXCL)) { in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->fscrypt_auth = *aux; in->mark_caps_dirty(CEPH_CAP_AUTH_EXCL); mask &= ~CEPH_SETATTR_FSCRYPT_AUTH; @@ -8202,8 +8170,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, stx->stx_size >= in->size) { if (stx->stx_size > in->size) { in->size = in->reported_size = stx->stx_size; - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->mark_caps_dirty(CEPH_CAP_FILE_EXCL); mask &= ~(CEPH_SETATTR_SIZE); mask |= CEPH_SETATTR_MTIME; @@ -8224,8 +8190,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!do_sync && in->caps_issued_mask(CEPH_CAP_FILE_EXCL)) { in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->fscrypt_file = *aux; in->mark_caps_dirty(CEPH_CAP_FILE_EXCL); mask &= ~CEPH_SETATTR_FSCRYPT_FILE; @@ -8241,8 +8205,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!do_sync && in->caps_issued_mask(CEPH_CAP_FILE_EXCL)) { in->mtime = utime_t(stx->stx_mtime); in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->time_warp_seq++; in->mark_caps_dirty(CEPH_CAP_FILE_EXCL); mask &= ~CEPH_SETATTR_MTIME; @@ -8250,8 +8212,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, utime_t(stx->stx_mtime) > in->mtime) { in->mtime = utime_t(stx->stx_mtime); in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->mark_caps_dirty(CEPH_CAP_FILE_WR); mask &= ~CEPH_SETATTR_MTIME; } else if (!in->caps_issued_mask(CEPH_CAP_FILE_SHARED) || @@ -8268,8 +8228,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, if (!do_sync && in->caps_issued_mask(CEPH_CAP_FILE_EXCL)) { in->atime = utime_t(stx->stx_atime); in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->time_warp_seq++; in->mark_caps_dirty(CEPH_CAP_FILE_EXCL); mask &= ~CEPH_SETATTR_ATIME; @@ -8277,8 +8235,6 @@ int Client::_do_setattr(Inode *in, struct ceph_statx *stx, int mask, utime_t(stx->stx_atime) > in->atime) { in->atime = utime_t(stx->stx_atime); in->ctime = ceph_clock_now(); - in->cap_dirtier_uid = perms.uid(); - in->cap_dirtier_gid = perms.gid(); in->mark_caps_dirty(CEPH_CAP_FILE_WR); mask &= ~CEPH_SETATTR_ATIME; } else if (!in->caps_issued_mask(CEPH_CAP_FILE_SHARED) || diff --git a/src/client/Inode.h b/src/client/Inode.h index f330824116530..a2f6c15beda20 100644 --- a/src/client/Inode.h +++ b/src/client/Inode.h @@ -97,9 +97,6 @@ struct CapSnap { bool writing = false, dirty_data = false; uint64_t flush_tid = 0; - int64_t cap_dirtier_uid = -1; - int64_t cap_dirtier_gid = -1; - explicit CapSnap(Inode *i) : in(i) {} -- 2.39.5