From 4991f179f6794d9d64d69fbb39338e738522881a Mon Sep 17 00:00:00 2001 From: Li Wei Date: Fri, 28 Nov 2014 17:15:04 +0800 Subject: [PATCH] LU-6027 mdd: Don't list "trusted.link" for orphans Commit 09fe679 prevents mdd_xattr_get() from accessing XATTR_NAME_LINK EAs of orphan objects but leaves mdd_xattr_list() unchanged. This is problematic for fgetxattr() calls on orphan objects when EA cache refills are involved. To respond to such a refill, mdt lists all of the object's EA names and gets the value for each name. In the case of an orphan, XATTR_NAME_LINK EA will be listed, but corresponding mdd_xattr_get() call will return -ENOENT, causing the fgetxattr() call to fail eventually. This patch updates mdd_xattr_list() to filter out XATTR_NAME_LINK EAs for orphan objects as well, following commit 09fe679. Change-Id: Ie59e4c9342056bacf57a82cf9bf77cfdedc99f6d Signed-off-by: Li Wei Reviewed-on: http://review.whamcloud.com/12884 Reviewed-by: Fan Yong Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Johann Lombardi --- lustre/mdd/mdd_object.c | 30 +++++++++++++++- lustre/tests/Makefile.am | 2 +- lustre/tests/multiop.c | 18 ++++++++++ lustre/tests/orphan_linkea_check.c | 72 ++++++++++++++++++++++++++++++++++++++ lustre/tests/sanity.sh | 5 +++ lustre/tests/sanityn.sh | 21 +++++++++++ 6 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 lustre/tests/orphan_linkea_check.c diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index f1d78f1..95d5313 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -289,7 +289,35 @@ static int mdd_xattr_list(const struct lu_env *env, struct md_object *obj, rc = mdo_xattr_list(env, mdd_obj, buf, mdd_object_capa(env, mdd_obj)); mdd_read_unlock(env, mdd_obj); - RETURN(rc); + if (rc < 0) + RETURN(rc); + + /* + * Filter out XATTR_NAME_LINK if this is an orphan object. See + * mdd_xattr_get(). + */ + if (unlikely(mdd_obj->mod_flags & (DEAD_OBJ | ORPHAN_OBJ))) { + char *end = (char *)buf->lb_buf + rc; + char *p = buf->lb_buf; + + while (p < end) { + char *next = p + strlen(p) + 1; + + if (strcmp(p, XATTR_NAME_LINK) == 0) { + if (end - next > 0) + memmove(p, next, end - next); + rc -= next - p; + CDEBUG(D_INFO, "Filtered out "XATTR_NAME_LINK + " of orphan "DFID"\n", + PFID(mdd_object_fid(mdd_obj))); + break; + } + + p = next; + } + } + + RETURN(rc); } int mdd_declare_object_create_internal(const struct lu_env *env, diff --git a/lustre/tests/Makefile.am b/lustre/tests/Makefile.am index c7284e1..1116aa7 100644 --- a/lustre/tests/Makefile.am +++ b/lustre/tests/Makefile.am @@ -74,7 +74,7 @@ noinst_PROGRAMS += openfilleddirunlink rename_many memhog noinst_PROGRAMS += mmap_sanity writemany reads flocks_test flock_deadlock noinst_PROGRAMS += write_time_limit rwv lgetxattr_size_check checkfiemap noinst_PROGRAMS += listxattr_size_check check_fhandle_syscalls badarea_io -noinst_PROGRAMS += llapi_layout_test +noinst_PROGRAMS += llapi_layout_test orphan_linkea_check bin_PROGRAMS = mcreate munlink testdir = $(libdir)/lustre/tests diff --git a/lustre/tests/multiop.c b/lustre/tests/multiop.c index fc4aa51..2152f4c 100644 --- a/lustre/tests/multiop.c +++ b/lustre/tests/multiop.c @@ -52,6 +52,7 @@ #include #include #include +#include #include #include @@ -63,10 +64,13 @@ char *buf, *buf_align; int bufsize = 0; sem_t sem; #define ALIGN_LEN 65535 +#define XATTR "user.multiop" char usage[] = "Usage: %s filename command-sequence [path...]\n" " command-sequence items:\n" +" A fsetxattr(\"user.multiop\")\n" +" a fgetxattr(\"user.multiop\")\n" " c close\n" " B[num] call setstripe ioctl to create stripes\n" " C[num] create with optional stripes\n" @@ -247,6 +251,20 @@ int main(int argc, char **argv) ts.tv_nsec = 0; while (sem_timedwait(&sem, &ts) < 0 && errno == EINTR); break; + case 'A': + if (fsetxattr(fd, XATTR, "multiop", 8, 0)) { + save_errno = errno; + perror("fsetxattr"); + exit(save_errno); + } + break; + case 'a': + if (fgetxattr(fd, XATTR, NULL, 0) == -1) { + save_errno = errno; + perror("fgetxattr"); + exit(save_errno); + } + break; case 'c': if (close(fd) == -1) { save_errno = errno; diff --git a/lustre/tests/orphan_linkea_check.c b/lustre/tests/orphan_linkea_check.c new file mode 100644 index 0000000..ecc3ca7 --- /dev/null +++ b/lustre/tests/orphan_linkea_check.c @@ -0,0 +1,72 @@ +/* + * Check that flistxattr() calls on orphans do not return "trusted.link" EAs. + */ + +#include +#include +#include +#include +#include +#include + +int +main(int argc, char *argv[]) +{ + char *names; + char *name; + ssize_t size; + int fd; + int rc; + + fd = open(argv[1], O_RDWR | O_CREAT, 0666); + if (fd == -1) { + perror("open"); + return 1; + } + + rc = unlink(argv[1]); + if (rc == -1) { + perror("unlink"); + close(fd); + return 1; + } + + size = flistxattr(fd, NULL, 0); + if (size == -1) { + perror("flistxattr size"); + close(fd); + return 1; + } + + names = malloc(size); + if (names == NULL) { + fprintf(stderr, "Cannot allocate names\n"); + close(fd); + return 1; + } + + size = flistxattr(fd, names, size); + if (size == -1) { + perror("flistxattr"); + free(names); + close(fd); + return 1; + } + + for (name = names; name < names + size; name += strlen(name) + 1) + if (strcmp(name, "trusted.link") == 0) { + free(names); + close(fd); + return 1; + } + + free(names); + + rc = close(fd); + if (rc == -1) { + perror("close"); + return 1; + } + + return 0; +} diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index cd2f4da..5cd926a 100644 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -6881,6 +6881,11 @@ test_102p() { # LU-4703 setxattr did not check ownership } run_test 102p "check setxattr(2) correctly fails without permission" +test_102q() { + orphan_linkea_check $DIR/$tfile || error "orphan_linkea_check" +} +run_test 102q "flistxattr should not return trusted.link EAs for orphans" + run_acl_subtest() { $LUSTRE/tests/acl/run $LUSTRE/tests/acl/$1.test diff --git a/lustre/tests/sanityn.sh b/lustre/tests/sanityn.sh index 4246365..0c20a69 100644 --- a/lustre/tests/sanityn.sh +++ b/lustre/tests/sanityn.sh @@ -2878,6 +2878,27 @@ test_81() { } run_test 81 "rename and stat under striped directory" +test_82() { + # Client 1 creates a file. + multiop_bg_pause $DIR1/$tfile O_ac || error "multiop_bg_pause 1" + pid1=$! + # Client 2 opens the file. + multiop_bg_pause $DIR2/$tfile o_Ac || error "multiop_bg_pause 2" + pid2=$! + # Client 1 makes the file an orphan. + rm $DIR1/$tfile || error "rm" + # Client 2 sets EA "user.multiop". + kill -s USR1 $pid2 + wait $pid2 || error "multiop 2" + # Client 1 gets EA "user.multiop". This used to fail because the EA + # cache refill would get "trusted.link" from mdd_xattr_list() but + # -ENOENT when trying to get "trusted.link"'s value. See also sanity + # 102q. + kill -s USR1 $pid1 + wait $pid1 || error "multiop 1" +} +run_test 82 "fsetxattr and fgetxattr on orphan files" + log "cleanup: ======================================================" [ "$(mount | grep $MOUNT2)" ] && umount $MOUNT2 -- 1.8.3.1