7529 SMB shares with no permission for root fail after restart

Reviewed by: Alek Pinchuk <alek@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
Reviewed by: Matt Barden <matt.barden@nexenta.com>
Reviewed by: Sanjay Nadkarni <sanjay.nadkarni@nexent.com>
Reviewed by: Saso Kiselkov <saso.kiselkov@nexenta.com>
Reviewed by: Rick McNeal <rick.mcneal@nexenta.com>
Approved by: Garrett D'Amore <garrett@damore.org>

1 files changed, 126 insertions, 38 deletions

diff --git a/usr/src/lib/smbsrv/libmlsvc/common/smb_share.c b/usr/src/lib/smbsrv/libmlsvc/common/smb_share.c
index 2471819c09..36d03752b2 100644
--- a/usr/src/lib/smbsrv/libmlsvc/common/smb_share.c
+++ b/usr/src/lib/smbsrv/libmlsvc/common/smb_share.c
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  *
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2015 Nexenta Systems, Inc. All rights reserved.
+ * Copyright 2016 Nexenta Systems, Inc. All rights reserved.
  */
 
 /*
@@ -389,6 +389,7 @@ smb_shr_add(smb_share_t *si)
 	struct stat st;
 	smb_share_t *cached_si;
 	nvlist_t *shrlist;
+	boolean_t created_zfs = B_FALSE;
 	uint32_t status;
 	int rc;
 
@@ -416,8 +417,34 @@ smb_shr_add(smb_share_t *si)
 		/*
 		 * If share type is STYPE_DISKTREE then the path to the
 		 * share should exist so that we can add the share to cache.
+		 * If path is ZFS, add the .zfs/shares/<share> entry.
+		 *
+		 * Both actions may require privileges that main dropped,
+		 * so we need to temporarily make those effective.
 		 */
-		rc = stat(si->shr_path, &st);
+		if (smb_proc_takesem() == 0) {
+
+			(void) priv_set(PRIV_ON, PRIV_EFFECTIVE,
+			    PRIV_FILE_DAC_READ,
+			    PRIV_FILE_DAC_SEARCH,
+			    PRIV_FILE_DAC_WRITE,
+			    NULL);
+
+			rc = stat(si->shr_path, &st);
+			if (rc == 0) {
+				smb_shr_zfs_add(si);
+				created_zfs = B_TRUE;
+			}
+
+			(void) priv_set(PRIV_OFF, PRIV_EFFECTIVE,
+			    PRIV_FILE_DAC_READ,
+			    PRIV_FILE_DAC_SEARCH,
+			    PRIV_FILE_DAC_WRITE,
+			    NULL);
+			smb_proc_givesem();
+		} else {
+			rc = NERR_InternalError;
+		}
 		if (rc != 0) {
 			smb_shr_cache_unlock();
 			return (NERR_ItemNotFound);
@@ -425,6 +452,7 @@ smb_shr_add(smb_share_t *si)
 	}
 
 	if ((status = smb_shr_cache_addent(si)) != NERR_Success) {
+		/* This error should be impossible after findent above. */
 		smb_shr_cache_unlock();
 		return (status);
 	}
@@ -440,9 +468,6 @@ smb_shr_add(smb_share_t *si)
 		if (rc == 0) {
 			smb_shr_publish(si->shr_name, si->shr_container);
 
-			/* If path is ZFS, add the .zfs/shares/<share> entry. */
-			smb_shr_zfs_add(si);
-
 			if ((si->shr_flags & SMB_SHRF_DFSROOT) != 0)
 				dfs_namespace_load(si->shr_name);
 
@@ -450,11 +475,34 @@ smb_shr_add(smb_share_t *si)
 		}
 	}
 
+	/*
+	 * Error code path, i.e. when the kernel could not accept
+	 * the new share for some reason.
+	 */
 	if (smb_shr_cache_lock(SMB_SHR_CACHE_WRLOCK) == NERR_Success) {
 		smb_shr_cache_delent(si->shr_name);
 		smb_shr_cache_unlock();
 	}
 
+	if (created_zfs && smb_proc_takesem() == 0) {
+
+		(void) priv_set(PRIV_ON, PRIV_EFFECTIVE,
+		    PRIV_FILE_DAC_READ,
+		    PRIV_FILE_DAC_SEARCH,
+		    PRIV_FILE_DAC_WRITE,
+		    NULL);
+
+		smb_shr_zfs_remove(si);
+
+		(void) priv_set(PRIV_OFF, PRIV_EFFECTIVE,
+		    PRIV_FILE_DAC_READ,
+		    PRIV_FILE_DAC_SEARCH,
+		    PRIV_FILE_DAC_WRITE,
+		    NULL);
+
+		smb_proc_givesem();
+	}
+
 	/*
 	 * rc == ENOENT means the shared directory doesn't exist
 	 */
@@ -505,9 +553,28 @@ smb_shr_remove(char *sharename)
 
 	/*
 	 * If path is ZFS, remove the .zfs/shares/<share> entry.  Need
-	 * to remove before cleanup of cache occurs.
+	 * to remove before cleanup of cache occurs.  These actions
+	 * require temporary elevation of privileges.
 	 */
-	smb_shr_zfs_remove(si);
+	if (smb_proc_takesem() == 0) {
+
+		(void) priv_set(PRIV_ON, PRIV_EFFECTIVE,
+		    PRIV_FILE_DAC_READ,
+		    PRIV_FILE_DAC_SEARCH,
+		    PRIV_FILE_DAC_WRITE,
+		    NULL);
+
+		smb_shr_zfs_remove(si);
+
+		(void) priv_set(PRIV_OFF, PRIV_EFFECTIVE,
+		    PRIV_FILE_DAC_READ,
+		    PRIV_FILE_DAC_SEARCH,
+		    PRIV_FILE_DAC_WRITE,
+		    NULL);
+
+		smb_proc_givesem();
+	}
+
 	(void) smb_shr_encode(si, &shrlist);
 
 	(void) strlcpy(container, si->shr_container, sizeof (container));
@@ -570,9 +637,25 @@ smb_shr_rename(char *from_name, char *to_name)
 	bcopy(from_si, &to_si, sizeof (smb_share_t));
 	(void) strlcpy(to_si.shr_name, to_name, sizeof (to_si.shr_name));
 
-
 	/* If path is ZFS, rename the .zfs/shares/<share> entry. */
-	smb_shr_zfs_rename(from_si, &to_si);
+	if (smb_proc_takesem() == 0) {
+
+		(void) priv_set(PRIV_ON, PRIV_EFFECTIVE,
+		    PRIV_FILE_DAC_READ,
+		    PRIV_FILE_DAC_SEARCH,
+		    PRIV_FILE_DAC_WRITE,
+		    NULL);
+
+		smb_shr_zfs_rename(from_si, &to_si);
+
+		(void) priv_set(PRIV_OFF, PRIV_EFFECTIVE,
+		    PRIV_FILE_DAC_READ,
+		    PRIV_FILE_DAC_SEARCH,
+		    PRIV_FILE_DAC_WRITE,
+		    NULL);
+
+		smb_proc_givesem();
+	}
 
 	if ((status = smb_shr_cache_addent(&to_si)) != NERR_Success) {
 		smb_shr_cache_unlock();
@@ -730,9 +813,25 @@ smb_shr_modify(smb_share_t *new_si)
 		smb_shr_publish(new_si->shr_name, new_si->shr_container);
 	}
 
-	if (quota_flag_changed) {
+	/* The following required privileges we dropped. */
+	if (quota_flag_changed && smb_proc_takesem() == 0) {
+
+		(void) priv_set(PRIV_ON, PRIV_EFFECTIVE,
+		    PRIV_FILE_DAC_READ,
+		    PRIV_FILE_DAC_SEARCH,
+		    PRIV_FILE_DAC_WRITE,
+		    NULL);
+
 		smb_shr_zfs_remove(&old_si);
 		smb_shr_zfs_add(si);
+
+		(void) priv_set(PRIV_OFF, PRIV_EFFECTIVE,
+		    PRIV_FILE_DAC_READ,
+		    PRIV_FILE_DAC_SEARCH,
+		    PRIV_FILE_DAC_WRITE,
+		    NULL);
+
+		smb_proc_givesem();
 	}
 
 	return (NERR_Success);
@@ -2049,8 +2148,8 @@ smb_shr_publisher_flush(list_t *lst)
 
 /*
  * If the share path refers to a ZFS file system, add the
- * .zfs/shares/<share> object and call smb_quota_add_fs()
- * to initialize quota support for the share.
+ * .zfs/shares/<share> object and add or remove the special
+ * directory and file telling clients about quota support.
  */
 static void
 smb_shr_zfs_add(smb_share_t *si)
@@ -2077,14 +2176,16 @@ smb_shr_zfs_add(smb_share_t *si)
 		syslog(LOG_INFO, "share: failed to add ACL object: %s: %s\n",
 		    si->shr_name, strerror(errno));
 
-	if ((si->shr_flags & SMB_SHRF_QUOTAS) != 0) {
-		ret = zfs_prop_get(zfshd, ZFS_PROP_MOUNTPOINT,
-		    buf, MAXPATHLEN, NULL, NULL, 0, B_FALSE);
-		if (ret != 0) {
-			syslog(LOG_INFO, "share: failed to get mountpoint: "
-			    "%s\n", si->shr_name);
-		} else {
+	ret = zfs_prop_get(zfshd, ZFS_PROP_MOUNTPOINT,
+	    buf, MAXPATHLEN, NULL, NULL, 0, B_FALSE);
+	if (ret != 0) {
+		syslog(LOG_INFO, "share: failed to get mountpoint: "
+		    "%s\n", si->shr_name);
+	} else {
+		if ((si->shr_flags & SMB_SHRF_QUOTAS) != 0) {
 			smb_quota_add_fs(buf);
+		} else {
+			smb_quota_remove_fs(buf);
 		}
 	}
 
@@ -2094,14 +2195,12 @@ smb_shr_zfs_add(smb_share_t *si)
 
 /*
  * If the share path refers to a ZFS file system, remove the
- * .zfs/shares/<share> object, and call smb_quota_remove_fs()
- * to end quota support for the share.
+ * .zfs/shares/<share> object.
  */
 static void
 smb_shr_zfs_remove(smb_share_t *si)
 {
 	libzfs_handle_t *libhd;
-	zfs_handle_t *zfshd;
 	int ret;
 	char buf[MAXPATHLEN];	/* dataset or mountpoint */
 
@@ -2111,29 +2210,18 @@ smb_shr_zfs_remove(smb_share_t *si)
 	if ((libhd = libzfs_init()) == NULL)
 		return;
 
-	if ((zfshd = zfs_open(libhd, buf, ZFS_TYPE_FILESYSTEM)) == NULL) {
-		libzfs_fini(libhd);
-		return;
-	}
-
 	errno = 0;
 	ret = zfs_smb_acl_remove(libhd, buf, si->shr_path, si->shr_name);
 	if (ret != 0 && errno != EAGAIN)
 		syslog(LOG_INFO, "share: failed to remove ACL object: %s: %s\n",
 		    si->shr_name, strerror(errno));
 
-	if ((si->shr_flags & SMB_SHRF_QUOTAS) != 0) {
-		ret = zfs_prop_get(zfshd, ZFS_PROP_MOUNTPOINT,
-		    buf, MAXPATHLEN, NULL, NULL, 0, B_FALSE);
-		if (ret != 0) {
-			syslog(LOG_INFO, "share: failed to get mountpoint: "
-			    "%s\n", si->shr_name);
-		} else {
-			smb_quota_remove_fs(buf);
-		}
-	}
+	/*
+	 * We could remove the quotas directory here, but that adds
+	 * significantly to the time required for a zpool export,
+	 * so just leave it here and fixup when we share next.
+	 */
 
-	zfs_close(zfshd);
 	libzfs_fini(libhd);
 }