Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 16 Aug 2018 18:58:34 +0000 (UTC)
From:      Kyle Evans <kevans@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r337924 - head/lib/libbe
Message-ID:  <201808161858.w7GIwYgG055526@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kevans
Date: Thu Aug 16 18:58:34 2018
New Revision: 337924
URL: https://svnweb.freebsd.org/changeset/base/337924

Log:
  libbe(3): Impose dataset length restrictions on boot env name validation
  
  Previously, we only validated names for character restrictions. This is
  helpful, but we should've also checked length restrictions- dataset names
  must be restricted to MAXNAMELEN.
  
  While here, move validation before doing a bunch of concatenations and fix
  error handling in be_rename. It was previously setting the error state based
  on return value from a libzfs function, which is wrong: libzfs errors don't
  necessarily match cleanly to libbe errors. This would cause the assertion in
  be_error to hit when the error was printed.

Modified:
  head/lib/libbe/be.c
  head/lib/libbe/libbe.3

Modified: head/lib/libbe/be.c
==============================================================================
--- head/lib/libbe/be.c	Thu Aug 16 18:44:50 2018	(r337923)
+++ head/lib/libbe/be.c	Thu Aug 16 18:58:34 2018	(r337924)
@@ -486,7 +486,7 @@ be_create_from_existing(libbe_handle_t *lbh, const cha
 	int err;
 	char buf[BE_MAXPATHLEN];
 
-	if ((err = be_snapshot(lbh, old, NULL, true, (char *)&buf)))
+	if ((err = be_snapshot(lbh, old, NULL, true, (char *)&buf)) != 0)
 		return (set_error(lbh, err));
 
 	err = be_create_from_existing_snap(lbh, name, (char *)buf);
@@ -577,11 +577,12 @@ be_root_concat(libbe_handle_t *lbh, const char *name, 
 
 /*
  * Verifies the validity of a boot environment name (A-Za-z0-9-_.). Returns
- * BE_ERR_SUCCESS (0) if name is valid, otherwise returns BE_ERR_INVALIDNAME.
+ * BE_ERR_SUCCESS (0) if name is valid, otherwise returns BE_ERR_INVALIDNAME
+ * or BE_ERR_PATHLEN.
  * Does not set internal library error state.
  */
 int
-be_validate_name(libbe_handle_t *lbh __unused, const char *name)
+be_validate_name(libbe_handle_t *lbh, const char *name)
 {
 	for (int i = 0; *name; i++) {
 		char c = *(name++);
@@ -590,6 +591,12 @@ be_validate_name(libbe_handle_t *lbh __unused, const c
 		return (BE_ERR_INVALIDNAME);
 	}
 
+	/*
+	 * Impose the additional restriction that the entire dataset name must
+	 * not exceed the maximum length of a dataset, i.e. MAXNAMELEN.
+	 */
+	if (strlen(lbh->root) + 1 + strlen(name) > MAXNAMELEN)
+		return (BE_ERR_PATHLEN);
 	return (BE_ERR_SUCCESS);
 }
 
@@ -605,14 +612,17 @@ be_rename(libbe_handle_t *lbh, const char *old, const 
 	zfs_handle_t *zfs_hdl;
 	int err;
 
+	/*
+	 * be_validate_name is documented not to set error state, so we should
+	 * do so here.
+	 */
+	if ((err = be_validate_name(lbh, new)) != 0)
+		return (set_error(lbh, err));
 	if ((err = be_root_concat(lbh, old, full_old)) != 0)
 		return (set_error(lbh, err));
 	if ((err = be_root_concat(lbh, new, full_new)) != 0)
 		return (set_error(lbh, err));
 
-	if ((err = be_validate_name(lbh, new)) != 0)
-		return (err);
-
 	/* Check if old is active BE */
 	if (strcmp(full_old, be_active_path(lbh)) == 0)
 		return (set_error(lbh, BE_ERR_MOUNTED));
@@ -639,8 +649,9 @@ be_rename(libbe_handle_t *lbh, const char *old, const 
 	err = zfs_rename(zfs_hdl, NULL, full_new, flags);
 
 	zfs_close(zfs_hdl);
-
-	return (set_error(lbh, err));
+	if (err != 0)
+		return (set_error(lbh, BE_ERR_UNKNOWN));
+	return (0);
 }
 
 

Modified: head/lib/libbe/libbe.3
==============================================================================
--- head/lib/libbe/libbe.3	Thu Aug 16 18:44:50 2018	(r337923)
+++ head/lib/libbe/libbe.3	Thu Aug 16 18:58:34 2018	(r337924)
@@ -331,7 +331,9 @@ environment name into
 .Pp
 The
 .Fn be_validate_name
-function will validate the given boot environment name.
+function will validate the given boot environment name for both length
+restrictions as well as valid character restrictions.
+This function does not set the internal library error state.
 .Pp
 The
 .Fn be_validate_snap



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808161858.w7GIwYgG055526>