From 873c4903a52d089cd8234b79d24f5a3fc3bccc82 Mon Sep 17 00:00:00 2001 From: Prakash Surya Date: Fri, 2 Sep 2016 21:33:34 -0700 Subject: [PATCH] 7336 vfork and O_CLOEXEC causes zfs_mount EBUSY Reviewed by: Matt Ahrens Reviewed by: Paul Dagnelie Reviewed by: Robert Mustacchi Approved by: Dan McDonald --- usr/src/lib/libzfs/common/libzfs_mount.c | 68 ++++++++++++++++++++-- usr/src/pkg/manifests/system-test-zfstest.mf | 2 + usr/src/test/zfs-tests/runfiles/delphix.run | 3 +- .../tests/functional/cli_root/zfs_mount/Makefile | 3 +- .../cli_root/zfs_mount/zfs_mount_012_neg.ksh | 50 ++++++++++++++++ 5 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 usr/src/test/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_neg.ksh diff --git a/usr/src/lib/libzfs/common/libzfs_mount.c b/usr/src/lib/libzfs/common/libzfs_mount.c index bc4dbf9c3d..7828e28f03 100644 --- a/usr/src/lib/libzfs/common/libzfs_mount.c +++ b/usr/src/lib/libzfs/common/libzfs_mount.c @@ -76,6 +76,7 @@ #include #include #include +#include #include @@ -171,13 +172,32 @@ is_shared(libzfs_handle_t *hdl, const char *mountpoint, zfs_share_proto_t proto) return (SHARED_NOT_SHARED); } -/* - * Returns true if the specified directory is empty. If we can't open the - * directory at all, return true so that the mount can fail with a more - * informative error message. - */ static boolean_t -dir_is_empty(const char *dirname) +dir_is_empty_stat(const char *dirname) +{ + struct stat st; + + /* + * We only want to return false if the given path is a non empty + * directory, all other errors are handled elsewhere. + */ + if (stat(dirname, &st) < 0 || !S_ISDIR(st.st_mode)) { + return (B_TRUE); + } + + /* + * An empty directory will still have two entries in it, one + * entry for each of "." and "..". + */ + if (st.st_size > 2) { + return (B_FALSE); + } + + return (B_TRUE); +} + +static boolean_t +dir_is_empty_readdir(const char *dirname) { DIR *dirp; struct dirent64 *dp; @@ -207,6 +227,42 @@ dir_is_empty(const char *dirname) } /* + * Returns true if the specified directory is empty. If we can't open the + * directory at all, return true so that the mount can fail with a more + * informative error message. + */ +static boolean_t +dir_is_empty(const char *dirname) +{ + struct statvfs64 st; + + /* + * If the statvfs call fails or the filesystem is not a ZFS + * filesystem, fall back to the slow path which uses readdir. + */ + if ((statvfs64(dirname, &st) != 0) || + (strcmp(st.f_basetype, "zfs") != 0)) { + return (dir_is_empty_readdir(dirname)); + } + + /* + * At this point, we know the provided path is on a ZFS + * filesystem, so we can use stat instead of readdir to + * determine if the directory is empty or not. We try to avoid + * using readdir because that requires opening "dirname"; this + * open file descriptor can potentially end up in a child + * process if there's a concurrent fork, thus preventing the + * zfs_mount() from otherwise succeeding (the open file + * descriptor inherited by the child process will cause the + * parent's mount to fail with EBUSY). The performance + * implications of replacing the open, read, and close with a + * single stat is nice; but is not the main motivation for the + * added complexity. + */ + return (dir_is_empty_stat(dirname)); +} + +/* * Checks to see if the mount is active. If the filesystem is mounted, we fill * in 'where' with the current mountpoint, and return 1. Otherwise, we return * 0. diff --git a/usr/src/pkg/manifests/system-test-zfstest.mf b/usr/src/pkg/manifests/system-test-zfstest.mf index 27c5874083..94c77083cb 100644 --- a/usr/src/pkg/manifests/system-test-zfstest.mf +++ b/usr/src/pkg/manifests/system-test-zfstest.mf @@ -633,6 +633,8 @@ file path=opt/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_010_neg \ mode=0555 file path=opt/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_011_neg \ mode=0555 +file path=opt/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_neg \ + mode=0555 file \ path=opt/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_all_001_pos \ mode=0555 diff --git a/usr/src/test/zfs-tests/runfiles/delphix.run b/usr/src/test/zfs-tests/runfiles/delphix.run index f9e8868cb3..b05ab5c382 100644 --- a/usr/src/test/zfs-tests/runfiles/delphix.run +++ b/usr/src/test/zfs-tests/runfiles/delphix.run @@ -136,7 +136,8 @@ tests = ['zfs_inherit_001_neg', 'zfs_inherit_002_neg', 'zfs_inherit_003_pos'] tests = ['zfs_mount_001_pos', 'zfs_mount_002_pos', 'zfs_mount_003_pos', 'zfs_mount_004_pos', 'zfs_mount_005_pos', 'zfs_mount_006_pos', 'zfs_mount_007_pos', 'zfs_mount_008_pos', 'zfs_mount_009_neg', - 'zfs_mount_010_neg', 'zfs_mount_011_neg', 'zfs_mount_all_001_pos'] + 'zfs_mount_010_neg', 'zfs_mount_011_neg', 'zfs_mount_012_neg', + 'zfs_mount_all_001_pos'] [/opt/zfs-tests/tests/functional/cli_root/zfs_promote] tests = ['zfs_promote_001_pos', 'zfs_promote_002_pos', 'zfs_promote_003_pos', diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile index 4b40b56b3b..4266b316a9 100644 --- a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile +++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_mount/Makefile @@ -10,7 +10,7 @@ # # -# Copyright (c) 2012 by Delphix. All rights reserved. +# Copyright (c) 2012, 2015 by Delphix. All rights reserved. # include $(SRC)/Makefile.master @@ -31,6 +31,7 @@ PROGS = cleanup \ zfs_mount_009_neg \ zfs_mount_010_neg \ zfs_mount_011_neg \ + zfs_mount_012_neg \ zfs_mount_all_001_pos FILES = zfs_mount.cfg \ diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_neg.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_neg.ksh new file mode 100644 index 0000000000..19fb3b2596 --- /dev/null +++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_mount/zfs_mount_012_neg.ksh @@ -0,0 +1,50 @@ +#!/bin/ksh -p +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright (c) 2015 by Delphix. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# Verify that zfs mount should fail with a non-empty directory +# +# STRATEGY: +# 1. Unmount the dataset +# 2. Create a new empty directory +# 3. Set the dataset's mountpoint +# 4. Attempt to mount the dataset +# 5. Verify the mount succeeds +# 6. Unmount the dataset +# 7. Create a file in the directory created in step 2 +# 8. Attempt to mount the dataset +# 9. Verify the mount fails +# + +verify_runnable "both" + +log_assert "zfs mount fails with non-empty directory" + +fs=$TESTPOOL/$TESTFS + +log_must zfs umount $fs +log_must mkdir -p $TESTDIR +log_must zfs set mountpoint=$TESTDIR $fs +log_must zfs mount $fs +log_must zfs umount $fs +log_must touch $TESTDIR/testfile.$$ +log_mustnot zfs mount $fs +log_must rm -rf $TESTDIR + +log_pass "zfs mount fails non-empty directory as expected." -- 2.11.4.GIT