From be8bca08d26ab603dba20f16668f8c06f2c2c25f Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Thu, 19 Jan 2023 18:07:27 +0100 Subject: [PATCH] LU-16494 fileset: check fileset for operations by fid Some operations by FID, such as lfs rmfid, must be aware of subdirectory mount (fileset) so that they do not operate on files that are outside of the namespace currently mounted by the client. For lfs rmfid, we first proceed to a fid2path resolution. As fid2path is already fileset aware, it fails if a file or a link to a file is outside of the subdirectory mount. So we carry on with rmfid only for FIDs for which the file and all links do appear under the current fileset. This new behavior is enabled as soon as we detect a subdirectory mount is done (either directly or imposed by a nodemap fileset). This means the new behavior does not impact normal, whole-namespace client mount. sanity test_421h is added to exercise this new capability. Lustre-change: https://review.whamcloud.com/49696 Lustre-commit: 9a72c073d33b0454229402c0cc930dc4e796107b Signed-off-by: Sebastien Buisson Change-Id: I47136ac0a3324b9afdd01b0f902abc37938bd361 Reviewed-by: Andreas Dilger Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/50072 Tested-by: jenkins Tested-by: Maloo --- lustre/include/lustre_export.h | 1 + lustre/llite/dir.c | 78 ++++++++++++++++++++++++++++++++++++++++++ lustre/llite/file.c | 54 +++++++++++++++++------------ lustre/llite/llite_internal.h | 2 ++ lustre/mdt/mdt_handler.c | 13 +++++++ lustre/obdclass/genops.c | 3 ++ lustre/tests/sanity.sh | 65 +++++++++++++++++++++++++++++++++++ 7 files changed, 194 insertions(+), 22 deletions(-) diff --git a/lustre/include/lustre_export.h b/lustre/include/lustre_export.h index fe55164..67d496a 100644 --- a/lustre/include/lustre_export.h +++ b/lustre/include/lustre_export.h @@ -312,6 +312,7 @@ struct obd_export { */ __u64 exp_last_xid; long *exp_used_slots; + struct lu_fid exp_root_fid; /* subdir mount fid */ }; #define exp_target_data u.eu_target_data diff --git a/lustre/llite/dir.c b/lustre/llite/dir.c index 79dfaba..f357941 100644 --- a/lustre/llite/dir.c +++ b/lustre/llite/dir.c @@ -1337,6 +1337,8 @@ out: int ll_rmfid(struct file *file, void __user *arg) { const struct fid_array __user *ufa = arg; + struct inode *inode = file_inode(file); + struct ll_sb_info *sbi = ll_i2sbi(inode); struct fid_array *lfa = NULL; size_t size; unsigned nr; @@ -1364,8 +1366,84 @@ int ll_rmfid(struct file *file, void __user *arg) if (copy_from_user(lfa, arg, size)) GOTO(free_rcs, rc = -EFAULT); + /* In case of subdirectory mount, we need to make sure all the files + * for which we want to remove FID are visible in the namespace. + */ + if (!fid_is_root(&sbi->ll_root_fid)) { + struct fid_array *lfa_new = NULL; + int path_len = PATH_MAX, linkno; + struct getinfo_fid2path *gf; + int idx, last_idx = nr - 1; + + OBD_ALLOC(lfa_new, size); + if (!lfa_new) + GOTO(free_rcs, rc = -ENOMEM); + lfa_new->fa_nr = 0; + + gf = kmalloc(sizeof(*gf) + path_len + 1, GFP_NOFS); + if (!gf) + GOTO(free_rcs, rc = -ENOMEM); + + for (idx = 0; idx < nr; idx++) { + linkno = 0; + while (1) { + memset(gf, 0, sizeof(*gf) + path_len + 1); + gf->gf_fid = lfa->fa_fids[idx]; + gf->gf_pathlen = path_len; + gf->gf_linkno = linkno; + rc = __ll_fid2path(inode, gf, + sizeof(*gf) + gf->gf_pathlen, + gf->gf_pathlen); + if (rc == -ENAMETOOLONG) { + struct getinfo_fid2path *tmpgf; + + path_len += PATH_MAX; + tmpgf = krealloc(gf, + sizeof(*gf) + path_len + 1, + GFP_NOFS); + if (!tmpgf) { + kfree(gf); + OBD_FREE(lfa_new, size); + GOTO(free_rcs, rc = -ENOMEM); + } + gf = tmpgf; + continue; + } + if (rc) + break; + if (gf->gf_linkno == linkno) + break; + linkno = gf->gf_linkno; + } + + if (!rc) { + /* All the links for this fid are visible in the + * mounted subdir. So add it to the list of fids + * to remove. + */ + lfa_new->fa_fids[lfa_new->fa_nr++] = + lfa->fa_fids[idx]; + } else { + /* At least one link for this fid is not visible + * in the mounted subdir. So add it at the end + * of the list that will be hidden to lower + * layers, and set -ENOENT as ret code. + */ + lfa_new->fa_fids[last_idx] = lfa->fa_fids[idx]; + rcs[last_idx--] = rc; + } + } + kfree(gf); + OBD_FREE(lfa, size); + lfa = lfa_new; + } + + if (lfa->fa_nr == 0) + GOTO(free_rcs, rc = rcs[nr - 1]); + /* Call mdc_iocontrol */ rc = md_rmfid(ll_i2mdexp(file_inode(file)), lfa, rcs, NULL); + lfa->fa_nr = nr; if (!rc) { for (i = 0; i < nr; i++) if (rcs[i]) diff --git a/lustre/llite/file.c b/lustre/llite/file.c index fdb05ae..c7783f2 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -3036,9 +3036,37 @@ lookup: return rc; } -int ll_fid2path(struct inode *inode, void __user *arg) +int __ll_fid2path(struct inode *inode, struct getinfo_fid2path *gfout, + size_t outsize, __u32 pathlen_orig) { struct obd_export *exp = ll_i2mdexp(inode); + int rc; + + /* Append root FID after gfout to let MDT know the root FID so that + * it can lookup the correct path, this is mainly for fileset. + * old server without fileset mount support will ignore this. + */ + *gfout->gf_u.gf_root_fid = *ll_inode2fid(inode); + + /* Call mdc_iocontrol */ + rc = obd_iocontrol(OBD_IOC_FID2PATH, exp, outsize, gfout, NULL); + + if (!rc && gfout->gf_pathlen && gfout->gf_u.gf_path[0] == '/') { + /* by convention, server side (mdt_path_current()) puts + * a leading '/' to tell client that we are dealing with + * an encrypted file + */ + rc = fid2path_for_enc_file(inode, gfout->gf_u.gf_path, + gfout->gf_pathlen); + if (!rc && strlen(gfout->gf_u.gf_path) > pathlen_orig) + rc = -EOVERFLOW; + } + + return rc; +} + +int ll_fid2path(struct inode *inode, void __user *arg) +{ const struct getinfo_fid2path __user *gfin = arg; __u32 pathlen, pathlen_orig; struct getinfo_fid2path *gfout; @@ -3067,30 +3095,12 @@ gf_alloc: if (copy_from_user(gfout, arg, sizeof(*gfout))) GOTO(gf_free, rc = -EFAULT); - /* append root FID after gfout to let MDT know the root FID so that it - * can lookup the correct path, this is mainly for fileset. - * old server without fileset mount support will ignore this. */ - *gfout->gf_u.gf_root_fid = *ll_inode2fid(inode); - gfout->gf_pathlen = pathlen; - /* Call mdc_iocontrol */ - rc = obd_iocontrol(OBD_IOC_FID2PATH, exp, outsize, gfout, NULL); - if (rc != 0) + gfout->gf_pathlen = pathlen; + rc = __ll_fid2path(inode, gfout, outsize, pathlen_orig); + if (rc) GOTO(gf_free, rc); - if (gfout->gf_pathlen && gfout->gf_u.gf_path[0] == '/') { - /* by convention, server side (mdt_path_current()) puts - * a leading '/' to tell client that we are dealing with - * an encrypted file - */ - rc = fid2path_for_enc_file(inode, gfout->gf_u.gf_path, - gfout->gf_pathlen); - if (rc) - GOTO(gf_free, rc); - if (strlen(gfout->gf_u.gf_path) > gfin->gf_pathlen) - GOTO(gf_free, rc = -EOVERFLOW); - } - if (copy_to_user(arg, gfout, sizeof(*gfout) + pathlen_orig)) rc = -EFAULT; diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h index 289f076..86387fdf 100644 --- a/lustre/llite/llite_internal.h +++ b/lustre/llite/llite_internal.h @@ -1317,6 +1317,8 @@ int ll_fsync(struct file *file, loff_t start, loff_t end, int data); int ll_merge_attr(const struct lu_env *env, struct inode *inode); int ll_merge_attr_try(const struct lu_env *env, struct inode *inode); int ll_fid2path(struct inode *inode, void __user *arg); +int __ll_fid2path(struct inode *inode, struct getinfo_fid2path *gfout, + size_t outsize, __u32 pathlen_orig); int ll_data_version(struct inode *inode, __u64 *data_version, int flags); int ll_hsm_release(struct inode *inode); int ll_hsm_state_set(struct inode *inode, struct hsm_state_set *hss); diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index ab9e9f1..5aa873c 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -459,6 +459,7 @@ static int mdt_get_root(struct tgt_session_info *tsi) } else { repbody->mbo_fid1 = mdt->mdt_md_root_fid; } + exp->exp_root_fid = repbody->mbo_fid1; repbody->mbo_valid |= OBD_MD_FLID; EXIT; @@ -7299,6 +7300,18 @@ static int mdt_fid2path(struct mdt_thread_info *info, RETURN(-EINVAL); } + /* return error if client-provided root fid + * is not the one stored in the export + */ + if (root_fid && !fid_is_zero(&info->mti_exp->exp_root_fid) && + !lu_fid_eq(root_fid, &info->mti_exp->exp_root_fid)) { + CDEBUG(D_INFO, + "%s: root fid from client "DFID" but "DFID" stored in export\n", + mdt_obd_name(mdt), PFID(root_fid), + PFID(&info->mti_exp->exp_root_fid)); + RETURN(-EXDEV); + } + obj = mdt_object_find(info->mti_env, mdt, &fp->gf_fid); if (IS_ERR(obj)) { rc = PTR_ERR(obj); diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index 481d87c..35bd981 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -1065,6 +1065,9 @@ struct obd_export *__class_new_export(struct obd_device *obd, obd_init_export(export); at_init(&export->exp_bl_lock_at, obd_timeout, 0); + export->exp_root_fid.f_seq = 0; + export->exp_root_fid.f_oid = 0; + export->exp_root_fid.f_ver = 0; spin_lock(&obd->obd_dev_lock); if (!obd_uuid_equals(cluuid, &obd->obd_uuid)) { diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 11a5ba6..b908453 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -26519,6 +26519,71 @@ test_421g() { } run_test 421g "rmfid to return errors properly" +test_421h() { + local mount_other + local mount_ret + local rmfid_ret + local old_fid + local fidA + local fidB + local fidC + local fidD + + (( MDS1_VERSION >= $(version_code 2.14.0.75) )) || + skip "Need MDS version at least 2.14.0.75" + + test_mkdir $DIR/$tdir + test_mkdir $DIR/$tdir/subdir + touch $DIR/$tdir/subdir/file0 + old_fid=$(lfs path2fid $DIR/$tdir/subdir/file0 | sed "s/[/][^:]*://g") + echo File $DIR/$tdir/subdir/file0 FID $old_fid + rm -f $DIR/$tdir/subdir/file0 + touch $DIR/$tdir/subdir/fileA + fidA=$(lfs path2fid $DIR/$tdir/subdir/fileA | sed "s/[/][^:]*://g") + echo File $DIR/$tdir/subdir/fileA FID $fidA + touch $DIR/$tdir/subdir/fileB + fidB=$(lfs path2fid $DIR/$tdir/subdir/fileB | sed "s/[/][^:]*://g") + echo File $DIR/$tdir/subdir/fileB FID $fidB + ln $DIR/$tdir/subdir/fileB $DIR/$tdir/subdir/fileB_hl + touch $DIR/$tdir/subdir/fileC + fidC=$(lfs path2fid $DIR/$tdir/subdir/fileC | sed "s/[/][^:]*://g") + echo File $DIR/$tdir/subdir/fileC FID $fidC + ln $DIR/$tdir/subdir/fileC $DIR/$tdir/fileC + touch $DIR/$tdir/fileD + fidD=$(lfs path2fid $DIR/$tdir/fileD | sed "s/[/][^:]*://g") + echo File $DIR/$tdir/fileD FID $fidD + + # mount another client mount point with subdirectory mount + export FILESET=/$tdir/subdir + mount_other=${MOUNT}_other + mount_client $mount_other ${MOUNT_OPTS} + mount_ret=$? + export FILESET="" + (( mount_ret == 0 )) || error "mount $mount_other failed" + + echo Removing FIDs: + echo $LFS rmfid $mount_other $old_fid $fidA $fidD $fidB $fidC + $LFS rmfid $mount_other $old_fid $fidA $fidD $fidB $fidC + rmfid_ret=$? + + umount_client $mount_other || error "umount $mount_other failed" + + (( rmfid_ret != 0 )) || error "rmfid should have failed" + + # fileA should have been deleted + stat $DIR/$tdir/subdir/fileA && error "fileA not deleted" + + # fileB should have been deleted + stat $DIR/$tdir/subdir/fileB && error "fileB not deleted" + + # fileC should not have been deleted, fid also exists outside of fileset + stat $DIR/$tdir/subdir/fileC || error "fileC deleted" + + # fileD should not have been deleted, it exists outside of fileset + stat $DIR/$tdir/fileD || error "fileD deleted" +} +run_test 421h "rmfid with fileset mount" + test_422() { test_mkdir -i 0 -c 1 -p $DIR/$tdir/d1 test_mkdir -i 0 -c 1 -p $DIR/$tdir/d2 -- 1.8.3.1