From 077570483e75e0610fd45149b926097547c434b8 Mon Sep 17 00:00:00 2001 From: Alexey Lyashkov Date: Wed, 8 Aug 2018 07:47:51 +0300 Subject: [PATCH] LU-11111 lfsck: skip orphan processing LFSCK can reconnect a recently-deleted orphan object back into the normal namespace when it shouldn't. This can cause access to the deleted data (potential security risk), and sometimes cause an assertion if orphan is later deleted. Skip LFSCK on orphan objects. Fix the handling of the LUSTRE_ORPHAN_FL in both osd-zfs and osd-ldiskfs so that la_valid |= LA_FLAGS is set when la_flags |= LUSTRE_ORPHAN_FL is set, otherwise the upper layers cannot properly detect it. Clean up alignment of flags values and provide hex equivalents so that they are more easily referenced during debugging. Signed-off-by: Alexey Lyashkov Signed-off-by: Andreas Dilger Change-Id: I9b2809b95efa4b3c3e3b2c7d0a501624ed743ede Reviewed-on: https://review.whamcloud.com/32959 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Lai Siyao Reviewed-by: Oleg Drokin --- lustre/include/uapi/linux/lustre/lustre_disk.h | 1 + lustre/include/uapi/linux/lustre/lustre_idl.h | 28 +++++++++---------- lustre/include/uapi/linux/lustre/lustre_user.h | 38 +++++++++++++------------- lustre/lfsck/lfsck_engine.c | 15 ++++++++-- lustre/mdd/mdd_object.c | 3 +- lustre/mdd/mdd_orphans.c | 2 +- lustre/osd-ldiskfs/osd_handler.c | 7 +++-- lustre/osd-ldiskfs/osd_scrub.c | 4 +-- lustre/osd-zfs/osd_object.c | 12 ++++++-- lustre/osd-zfs/osd_scrub.c | 2 +- lustre/tests/sanity-lfsck.sh | 23 ++++++++++++++++ 11 files changed, 88 insertions(+), 47 deletions(-) diff --git a/lustre/include/uapi/linux/lustre/lustre_disk.h b/lustre/include/uapi/linux/lustre/lustre_disk.h index 9acc3ef..8565441 100644 --- a/lustre/include/uapi/linux/lustre/lustre_disk.h +++ b/lustre/include/uapi/linux/lustre/lustre_disk.h @@ -71,6 +71,7 @@ #define LFSCK_NAMESPACE "lfsck_namespace" #define REMOTE_PARENT_DIR "REMOTE_PARENT_DIR" #define INDEX_BACKUP_DIR "index_backup" +#define MDT_ORPHAN_DIR "PENDING" /****************** persistent mount data *********************/ diff --git a/lustre/include/uapi/linux/lustre/lustre_idl.h b/lustre/include/uapi/linux/lustre/lustre_idl.h index 71e9af1..f7d9a01 100644 --- a/lustre/include/uapi/linux/lustre/lustre_idl.h +++ b/lustre/include/uapi/linux/lustre/lustre_idl.h @@ -1717,17 +1717,17 @@ enum { enum { /* these should be identical to their EXT4_*_FL counterparts, they are * redefined here only to avoid dragging in fs/ext4/ext4.h */ - LUSTRE_SYNC_FL = 0x00000008, /* Synchronous updates */ - LUSTRE_IMMUTABLE_FL = 0x00000010, /* Immutable file */ - LUSTRE_APPEND_FL = 0x00000020, /* writes to file may only append */ - LUSTRE_NODUMP_FL = 0x00000040, /* do not dump file */ - LUSTRE_NOATIME_FL = 0x00000080, /* do not update atime */ - LUSTRE_INDEX_FL = 0x00001000, /* hash-indexed directory */ - LUSTRE_DIRSYNC_FL = 0x00010000, /* dirsync behaviour (dir only) */ - LUSTRE_TOPDIR_FL = 0x00020000, /* Top of directory hierarchies*/ - LUSTRE_DIRECTIO_FL = 0x00100000, /* Use direct i/o */ - LUSTRE_INLINE_DATA_FL = 0x10000000, /* Inode has inline data. */ - LUSTRE_PROJINHERIT_FL = 0x20000000, /* Create with parents projid */ + LUSTRE_SYNC_FL = 0x00000008, /* Synchronous updates */ + LUSTRE_IMMUTABLE_FL = 0x00000010, /* Immutable file */ + LUSTRE_APPEND_FL = 0x00000020, /* file writes may only append */ + LUSTRE_NODUMP_FL = 0x00000040, /* do not dump file */ + LUSTRE_NOATIME_FL = 0x00000080, /* do not update atime */ + LUSTRE_INDEX_FL = 0x00001000, /* hash-indexed directory */ + LUSTRE_DIRSYNC_FL = 0x00010000, /* dirsync behaviour (dir only) */ + LUSTRE_TOPDIR_FL = 0x00020000, /* Top of directory hierarchies*/ + LUSTRE_DIRECTIO_FL = 0x00100000, /* Use direct i/o */ + LUSTRE_INLINE_DATA_FL = 0x10000000, /* Inode has inline data. */ + LUSTRE_PROJINHERIT_FL = 0x20000000, /* Create with parents projid */ /* These flags will not be identical to any EXT4_*_FL counterparts, * and only reserved for lustre purpose. Note: these flags might @@ -1736,10 +1736,10 @@ enum { * wired by la_flags see osd_attr_get(). * 2. If these flags needs to be stored into inode, they will be * stored in LMA. see LMAI_XXXX */ - LUSTRE_ORPHAN_FL = 0x00002000, - LUSTRE_SET_SYNC_FL = 0x00040000, /* Synchronous setattr on OSTs */ + LUSTRE_ORPHAN_FL = 0x00002000, + LUSTRE_SET_SYNC_FL = 0x00040000, /* Synchronous setattr on OSTs */ - LUSTRE_LMA_FL_MASKS = LUSTRE_ORPHAN_FL, + LUSTRE_LMA_FL_MASKS = LUSTRE_ORPHAN_FL, }; #ifndef FS_XFLAG_SYNC diff --git a/lustre/include/uapi/linux/lustre/lustre_user.h b/lustre/include/uapi/linux/lustre/lustre_user.h index 0b143cb..b75f0c6 100644 --- a/lustre/include/uapi/linux/lustre/lustre_user.h +++ b/lustre/include/uapi/linux/lustre/lustre_user.h @@ -1069,25 +1069,25 @@ struct lustre_swap_layouts { * Only the first 12 bits are currently saved. */ enum la_valid { - LA_ATIME = 1 << 0, - LA_MTIME = 1 << 1, - LA_CTIME = 1 << 2, - LA_SIZE = 1 << 3, - LA_MODE = 1 << 4, - LA_UID = 1 << 5, - LA_GID = 1 << 6, - LA_BLOCKS = 1 << 7, - LA_TYPE = 1 << 8, - LA_FLAGS = 1 << 9, - LA_NLINK = 1 << 10, - LA_RDEV = 1 << 11, - LA_BLKSIZE = 1 << 12, - LA_KILL_SUID = 1 << 13, - LA_KILL_SGID = 1 << 14, - LA_PROJID = 1 << 15, - LA_LAYOUT_VERSION = 1 << 16, - LA_LSIZE = 1 << 17, - LA_LBLOCKS = 1 << 18, + LA_ATIME = 1 << 0, /* 0x00001 */ + LA_MTIME = 1 << 1, /* 0x00002 */ + LA_CTIME = 1 << 2, /* 0x00004 */ + LA_SIZE = 1 << 3, /* 0x00008 */ + LA_MODE = 1 << 4, /* 0x00010 */ + LA_UID = 1 << 5, /* 0x00020 */ + LA_GID = 1 << 6, /* 0x00040 */ + LA_BLOCKS = 1 << 7, /* 0x00080 */ + LA_TYPE = 1 << 8, /* 0x00100 */ + LA_FLAGS = 1 << 9, /* 0x00200 */ + LA_NLINK = 1 << 10, /* 0x00400 */ + LA_RDEV = 1 << 11, /* 0x00800 */ + LA_BLKSIZE = 1 << 12, /* 0x01000 */ + LA_KILL_SUID = 1 << 13, /* 0x02000 */ + LA_KILL_SGID = 1 << 14, /* 0x04000 */ + LA_PROJID = 1 << 15, /* 0x08000 */ + LA_LAYOUT_VERSION = 1 << 16, /* 0x10000 */ + LA_LSIZE = 1 << 17, /* 0x20000 */ + LA_LBLOCKS = 1 << 18, /* 0x40000 */ /** * Attributes must be transmitted to OST objects */ diff --git a/lustre/lfsck/lfsck_engine.c b/lustre/lfsck/lfsck_engine.c index 626c2bb..4eb62d4 100644 --- a/lustre/lfsck/lfsck_engine.c +++ b/lustre/lfsck/lfsck_engine.c @@ -960,8 +960,19 @@ static int lfsck_master_oit_engine(const struct lu_env *env, goto checkpoint; } - if (dt_object_exists(target)) - rc = lfsck_exec_oit(env, lfsck, target); + if (dt_object_exists(target)) { + struct lu_attr la = { .la_valid = 0 }; + + rc = dt_attr_get(env, target, &la); + if (likely(!rc && (!(la.la_valid & LA_FLAGS) || + !(la.la_flags & LUSTRE_ORPHAN_FL)))) + rc = lfsck_exec_oit(env, lfsck, target); + else + CDEBUG(D_INFO, + "%s: orphan "DFID", %llx/%x: rc = %d\n", + lfsck_lfsck2name(lfsck), PFID(fid), + la.la_valid, la.la_flags, rc); + } lfsck_object_put(env, target); if (rc != 0 && bk->lb_param & LPF_FAILOUT) diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index ea8f1b7..ff99634 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -235,8 +235,7 @@ int mdd_la_get(const struct lu_env *env, struct mdd_object *obj, return rc; } - if (la->la_valid & LA_FLAGS && - la->la_flags & LUSTRE_ORPHAN_FL) + if (la->la_valid & LA_FLAGS && la->la_flags & LUSTRE_ORPHAN_FL) obj->mod_flags |= ORPHAN_OBJ | DEAD_OBJ; return 0; diff --git a/lustre/mdd/mdd_orphans.c b/lustre/mdd/mdd_orphans.c index 0b33f40..2de5baa 100644 --- a/lustre/mdd/mdd_orphans.c +++ b/lustre/mdd/mdd_orphans.c @@ -45,7 +45,7 @@ #include #include "mdd_internal.h" -static const char mdd_orphan_index_name[] = "PENDING"; +static const char mdd_orphan_index_name[] = MDT_ORPHAN_DIR; static const char dotdot[] = ".."; enum { diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index d03fb6b..73d0f02 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -2624,8 +2624,7 @@ static void osd_inode_getattr(const struct lu_env *env, attr->la_flags |= LUSTRE_PROJINHERIT_FL; } -static int osd_attr_get(const struct lu_env *env, - struct dt_object *dt, +static int osd_attr_get(const struct lu_env *env, struct dt_object *dt, struct lu_attr *attr) { struct osd_object *obj = osd_dt_obj(dt); @@ -2640,8 +2639,10 @@ static int osd_attr_get(const struct lu_env *env, spin_lock(&obj->oo_guard); osd_inode_getattr(env, obj->oo_inode, attr); - if (obj->oo_lma_flags & LUSTRE_ORPHAN_FL) + if (obj->oo_lma_flags & LUSTRE_ORPHAN_FL) { + attr->la_valid |= LA_FLAGS; attr->la_flags |= LUSTRE_ORPHAN_FL; + } spin_unlock(&obj->oo_guard); return 0; diff --git a/lustre/osd-ldiskfs/osd_scrub.c b/lustre/osd-ldiskfs/osd_scrub.c index 9281756..6232235 100644 --- a/lustre/osd-ldiskfs/osd_scrub.c +++ b/lustre/osd-ldiskfs/osd_scrub.c @@ -1426,8 +1426,8 @@ static const struct osd_lf_map osd_lf_maps[] = { /* PENDING */ { - .olm_name = "PENDING", - .olm_namelen = sizeof("PENDING") - 1, + .olm_name = MDT_ORPHAN_DIR, + .olm_namelen = sizeof(MDT_ORPHAN_DIR) - 1, }, /* ROOT */ diff --git a/lustre/osd-zfs/osd_object.c b/lustre/osd-zfs/osd_object.c index a04a29b..170d223 100644 --- a/lustre/osd-zfs/osd_object.c +++ b/lustre/osd-zfs/osd_object.c @@ -984,8 +984,7 @@ static int osd_write_locked(const struct lu_env *env, struct dt_object *dt) return rc; } -static int osd_attr_get(const struct lu_env *env, - struct dt_object *dt, +static int osd_attr_get(const struct lu_env *env, struct dt_object *dt, struct lu_attr *attr) { struct osd_object *obj = osd_dt_obj(dt); @@ -1006,9 +1005,16 @@ static int osd_attr_get(const struct lu_env *env, read_lock(&obj->oo_attr_lock); *attr = obj->oo_attr; - if (obj->oo_lma_flags & LUSTRE_ORPHAN_FL) + if (obj->oo_lma_flags & LUSTRE_ORPHAN_FL) { + attr->la_valid |= LA_FLAGS; attr->la_flags |= LUSTRE_ORPHAN_FL; + } read_unlock(&obj->oo_attr_lock); + if (attr->la_valid & LA_FLAGS && attr->la_flags & LUSTRE_ORPHAN_FL) + CDEBUG(D_INFO, "%s: set orphan flag on "DFID" (%llx/%x)\n", + osd_obj2dev(obj)->od_svname, + PFID(lu_object_fid(&dt->do_lu)), + attr->la_valid, obj->oo_lma_flags); /* with ZFS_DEBUG zrl_add_debug() called by DB_DNODE_ENTER() * from within sa_object_size() can block on a mutex, so diff --git a/lustre/osd-zfs/osd_scrub.c b/lustre/osd-zfs/osd_scrub.c index b8afa96..d3c3e90 100644 --- a/lustre/osd-zfs/osd_scrub.c +++ b/lustre/osd-zfs/osd_scrub.c @@ -701,7 +701,7 @@ static const struct osd_lf_map osd_lf_maps[] = { /* PENDING */ { - .olm_name = "PENDING", + .olm_name = MDT_ORPHAN_DIR, }, /* ROOT */ diff --git a/lustre/tests/sanity-lfsck.sh b/lustre/tests/sanity-lfsck.sh index 69f2783..4456975 100644 --- a/lustre/tests/sanity-lfsck.sh +++ b/lustre/tests/sanity-lfsck.sh @@ -5532,6 +5532,29 @@ test_36c() { } run_test 36c "rebuild LOV EA for mirrored file (3)" +test_37() +{ + local PID + local rc + local t_dir="$DIR/$tdir/d0" + check_mount_and_prep + + $LFS mkdir -i 0 $t_dir || error "(2) Fail to mkdir $t_dir on MDT0" + multiop_bg_pause $t_dir D_c || { error "multiop failed: $?"; return 1; } + PID=$! + rmdir $t_dir + + $START_NAMESPACE -r -A || { + error "(3) Fail to start LFSCK for namespace!"; kill -USR1 $PID; } + + wait_all_targets_blocked namespace completed 4 + stat $t_dir && rc=1 + kill -USR1 $PID + return $rc +} +run_test 37 "LFSCK must skip a ORPHAN" + + # restore MDS/OST size MDSSIZE=${SAVED_MDSSIZE} OSTSIZE=${SAVED_OSTSIZE} -- 1.8.3.1