From e603ddadd5eef4173079d20a0ec1b388b4935cc0 Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Tue, 20 Aug 2024 17:27:32 +0200 Subject: [PATCH] LU-18158 sec: hint client in case of failed reint open In case of failed file open, the Lustre client is already able to send alternative supplementary groups to the server using a retry mechanism. If the first attempt fails, then the failure reply from the server includes a hint with a possible supplementary group and ACLs. The client then retries with an alternative supplementary group, selected using information hinted in the reply from the MDT. The same mechanism can be implemented for all reint open cases. Signed-off-by: Sebastien Buisson Change-Id: Ic5e309cfbdb6d388397e3e251b2d007a612fd214 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/56098 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin Reviewed-by: Lai Siyao Reviewed-by: Andreas Dilger --- lustre/llite/namei.c | 17 +++++++++++++++-- lustre/mdt/mdt_open.c | 43 +++++++++++++++++++++++++++++++++++++++++-- lustre/tests/sanity-sec.sh | 15 +++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/lustre/llite/namei.c b/lustre/llite/namei.c index d8f4aba..4674448 100644 --- a/lustre/llite/namei.c +++ b/lustre/llite/namei.c @@ -893,6 +893,20 @@ static __u32 get_uc_group_from_acl(const struct posix_acl *acl, int want) return (__u32)__kgid_val(INVALID_GID); } +static bool failed_it_can_retry(int retval, struct lookup_intent *it) +{ + int rc = 0; + + if (!retval && (it->it_op & IT_OPEN_CREAT) == IT_OPEN_CREAT && + it_disposition(it, DISP_OPEN_CREATE)) { + rc = it_open_error(DISP_OPEN_CREATE, it); + } else { + rc = retval; + } + + return (rc == -EACCES); +} + /* This function implements a retry mechanism on top of md_intent_lock(). * This is useful because the client can provide at most 2 supplementary * groups in the request sent to the MDS, but sometimes it does not know @@ -921,8 +935,7 @@ intent: "intent lock %d on i1 "DFID" suppgids %d %d: rc %d\n", it->it_op, PFID(&op_data->op_fid1), op_data->op_suppgids[0], op_data->op_suppgids[1], rc); - if (rc == -EACCES && tryagain && it->it_op & IT_OPEN && - it_disposition(it, DISP_OPEN_DENY) && *reqp) { + if (tryagain && *reqp && failed_it_can_retry(rc, it)) { struct mdt_body *body; __u32 new_suppgid; diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c index c4c40d1..703bbd3 100644 --- a/lustre/mdt/mdt_open.c +++ b/lustre/mdt/mdt_open.c @@ -1365,6 +1365,34 @@ static inline enum ldlm_mode mdt_open_lock_mode(struct mdt_thread_info *info, return (result == 0) ? LCK_PR : LCK_PW; } +static void mdt_pack_attr_acl(struct mdt_thread_info *info, + struct mdt_object *o) +{ + struct ptlrpc_request *req = mdt_info_req(info); + struct md_attr *ma = &info->mti_attr; + struct lu_nodemap *nodemap; + struct mdt_body *repbody; + int rc; + + repbody = req_capsule_server_get(info->mti_pill, &RMF_MDT_BODY); + + rc = mdt_attr_get_complex(info, o, ma); + if (!rc) + mdt_pack_attr2body(info, repbody, &ma->ma_attr, + mdt_object_fid(o)); + +#ifdef CONFIG_LUSTRE_FS_POSIX_ACL + if (exp_connect_flags(req->rq_export) & OBD_CONNECT_ACL) { + nodemap = nodemap_get_from_exp(req->rq_export); + if (IS_ERR(nodemap)) + return; + + (void)mdt_pack_acl2body(info, repbody, o, nodemap); + nodemap_putref(nodemap); + } +#endif +} + int mdt_reint_open(struct mdt_thread_info *info, struct mdt_lock_handle *lhc) { struct mdt_device *mdt = info->mti_mdt; @@ -1506,8 +1534,14 @@ again_pw: PFID(mdt_object_fid(parent)), PNAME(&rr->rr_name), PFID(child_fid)); - if (result != 0 && result != -ENOENT) + if (result != 0 && result != -ENOENT) { + /* in case of 'permission denied', + * hint the client with supp groups and ACLs + */ + if (result == -EACCES) + mdt_pack_attr_acl(info, parent); GOTO(out_parent_unlock, result); + } CFS_RACE(OBD_FAIL_MDS_REINT_OPEN2); @@ -1588,8 +1622,13 @@ again_pw: if (result == 0) result = mdt_attr_get_complex(info, child, ma); - if (result != 0) + if (result != 0) { + /* in case of failure to create, + * hint the client with supp groups and ACLs + */ + mdt_pack_attr_acl(info, parent); GOTO(out_child, result); + } } created = 1; mdt_counter_incr(req, LPROC_MDT_MKNOD, diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh index 0bccf52..05f19d1 100755 --- a/lustre/tests/sanity-sec.sh +++ b/lustre/tests/sanity-sec.sh @@ -6346,6 +6346,7 @@ test_64g() { chmod 640 $DIR/$tdir/fileA chgrp grptest64g3 $DIR/$tdir/fileA setfacl -m g:grptest64g2:r $DIR/$tdir/fileA + setfacl -m g:grptest64g2:rwx $DIR/$tdir ls -lR $DIR/$tdir setup_64 @@ -6358,7 +6359,21 @@ test_64g() { error "setting rbac file_perms failed" wait_nm_sync c0 rbac + $RUNAS touch $DIR/$tdir/fileB && + error "touch $DIR/$tdir/fileB should fail" + do_nodes $(comma_list $(all_mdts_nodes)) \ + $LCTL set_param mdt.*.identity_int_flush=$RUNAS_ID + $RUNAS -G 5001 touch $DIR/$tdir/fileB || + error "touch $DIR/$tdir/fileB failed" + do_nodes $(comma_list $(all_mdts_nodes)) \ + $LCTL set_param mdt.*.identity_int_flush=$RUNAS_ID + $RUNAS -G 5000,5001 touch $DIR/$tdir/fileC || + error "touch $DIR/$tdir/fileC failed" + do_nodes $(comma_list $(all_mdts_nodes)) \ + $LCTL set_param mdt.*.identity_int_flush=$RUNAS_ID $RUNAS cat $DIR/$tdir/fileA && error "cat $DIR/$tdir/fileA should fail" + do_nodes $(comma_list $(all_mdts_nodes)) \ + $LCTL set_param mdt.*.identity_int_flush=$RUNAS_ID $RUNAS -G 5000,5001 cat $DIR/$tdir/fileA || error "cat $DIR/$tdir/fileA failed" } -- 1.8.3.1