bin/157691: [zfs] [patch] zpool import -d broken

Christopher Key cjk32 at cam.ac.uk
Tue Jun 7 17:30:09 UTC 2011


>Number:         157691
>Category:       bin
>Synopsis:       [zfs] [patch] zpool import -d broken
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Jun 07 17:30:09 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator:     Christopher Key
>Release:        FreeBSD 8.2-RC3 amd64
>Organization:
>Environment:
System: FreeBSD chacal.wzl33 8.2-RC3 FreeBSD 8.2-RC3 #6: Mon Feb 14 00:36:21 GMT 2011 root at chacal.wzl33:/usr/obj/usr/src/sys/CHACAL amd64


>Description:
zfs import -d <dir> is supposed to enumerate the contents of dir, opening and statting each item within before possible further processing.  However, the way in which the path to be opened and statted is built is broken.  The fundamental problem is repeated usage of,

snprintf(path, sizeof (path), "%s/%s", rdsk, dp->d_name)

where,

rdsk == path

>How-To-Repeat:
Without patch:

chris at chacal:~> ls -al /dev/mirror
total 1
dr-xr-xr-x   2 root  wheel          512  6 Jun 19:20 .
dr-xr-xr-x  11 root  wheel          512  6 Jun 19:20 ..
crw-r-----   1 root  operator    0, 108  6 Jun 19:20 gm0
crw-r-----   1 root  operator    0, 112  6 Jun 19:20 gm0p1
crw-r-----   1 root  operator    0, 113  6 Jun 19:20 gm0p2
crw-r-----   1 root  operator    0, 114  6 Jun 19:20 gm0p3
crw-r-----   1 root  operator    0, 115  6 Jun 19:20 gm0p4
crw-r-----   1 root  operator    0, 116  6 Jun 19:20 gm0p5
crw-r-----   1 root  operator    0, 117  6 Jun 19:20 gm0p6
crw-r-----   1 root  operator    0, 143  6 Jun 19:20 gm1
crw-r-----   1 root  operator    0, 189  6 Jun 19:20 gm1s1
crw-r-----   1 root  operator    0, 190  6 Jun 19:20 gm1s2
crw-r-----   1 root  operator    0, 191  6 Jun 19:20 gm1s3
chris at chacal:~> sudo truss zpool import -d /dev/mirror 2>&1 | grep /dev
open("/dev/zfs",O_RDWR,00) 	 	 = 3 (0x3)
open("/dev/zero",O_RDONLY,0666)		      = 4 (0x4)
lstat("/dev",{ mode=dr-xr-xr-x ,inode=2,size=512,blksize=4096 }) = 0 (0x0)
lstat("/dev/mirror",{ mode=dr-xr-xr-x ,inode=215,size=512,blksize=4096 }) = 0 (0x0)
stat("/dev/mirror/",{ mode=dr-xr-xr-x ,inode=215,size=512,blksize=4096 }) = 0 (0x0)
open("/dev/mirror/",O_NONBLOCK,01)    				        = 6 (0x6)
open("/dev/mirror//gm0",O_RDONLY,00)					   = 7 (0x7)
open("/dev/mirror//gm0/gm0p1",O_RDONLY,00)				    ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2",O_RDONLY,00) ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2/gm0p3",O_RDONLY,00) ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2/gm0p3/gm0p4",O_RDONLY,00) ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2/gm0p3/gm0p4/gm0p5",O_RDONLY,00) ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2/gm0p3/gm0p4/gm0p5/gm0p6",O_RDONLY,00) ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2/gm0p3/gm0p4/gm0p5/gm0p6/gm1",O_RDONLY,00) ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2/gm0p3/gm0p4/gm0p5/gm0p6/gm1/gm1s1",O_RDONLY,00) ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2/gm0p3/gm0p4/gm0p5/gm0p6/gm1/gm1s1/gm1s2",O_RDONLY,00) ERR#20 'Not a directory'
open("/dev/mirror//gm0/gm0p1/gm0p2/gm0p3/gm0p4/gm0p5/gm0p6/gm1/gm1s1/gm1s2/gm1s3",O_RDONLY,00) ERR#20 'Not a directory'


With patch:

chris at chacal:~> ls -al /dev/mirror
total 1
dr-xr-xr-x   2 root  wheel          512  6 Jun 19:20 .
dr-xr-xr-x  11 root  wheel          512  6 Jun 19:20 ..
crw-r-----   1 root  operator    0, 108  6 Jun 19:20 gm0
crw-r-----   1 root  operator    0, 112  6 Jun 19:20 gm0p1
crw-r-----   1 root  operator    0, 113  6 Jun 19:20 gm0p2
crw-r-----   1 root  operator    0, 114  6 Jun 19:20 gm0p3
crw-r-----   1 root  operator    0, 115  6 Jun 19:20 gm0p4
crw-r-----   1 root  operator    0, 116  6 Jun 19:20 gm0p5
crw-r-----   1 root  operator    0, 117  6 Jun 19:20 gm0p6
crw-r-----   1 root  operator    0, 143  6 Jun 19:20 gm1
crw-r-----   1 root  operator    0, 189  6 Jun 19:20 gm1s1
crw-r-----   1 root  operator    0, 190  6 Jun 19:20 gm1s2
crw-r-----   1 root  operator    0, 191  6 Jun 19:20 gm1s3
chris at chacal:~> sudo truss zpool import -d /dev/mirror 2>&1 | grep /dev
open("/dev/zfs",O_RDWR,00) 	 	 = 3 (0x3)
open("/dev/zero",O_RDONLY,0666)		      = 4 (0x4)
lstat("/dev",{ mode=dr-xr-xr-x ,inode=2,size=512,blksize=4096 }) = 0 (0x0)
lstat("/dev/mirror",{ mode=dr-xr-xr-x ,inode=215,size=512,blksize=4096 }) = 0 (0x0)
stat("/dev/mirror/",{ mode=dr-xr-xr-x ,inode=215,size=512,blksize=4096 }) = 0 (0x0)
open("/dev/mirror/",O_NONBLOCK,0144)  				        = 6 (0x6)
open("/dev/mirror/gm0",O_RDONLY,01760)					   = 7 (0x7)
open("/dev/mirror/gm0p1",O_RDONLY,01756)				    = 7 (0x7)
open("/dev/mirror/gm0p2",O_RDONLY,01756)				     = 7 (0x7)
open("/dev/mirror/gm0p3",O_RDONLY,01756)				      = 7 (0x7)
open("/dev/mirror/gm0p4",O_RDONLY,01756)				       = 7 (0x7)
open("/dev/mirror/gm0p5",O_RDONLY,01756)				        = 7 (0x7)
open("/dev/mirror/gm0p6",O_RDONLY,01756)					 = 7 (0x7)
open("/dev/mirror/gm1",O_RDONLY,01760)						    = 7 (0x7)
open("/dev/mirror/gm1s1",O_RDONLY,01756)					     = 7 (0x7)
open("/dev/mirror/gm1s2",O_RDONLY,01756)					      = 7 (0x7)
open("/dev/mirror/gm1s3",O_RDONLY,01756)					       = 7 (0x7)


>Fix:

	

--- zpool-import-d-patch begins here ---
Index: cddl/contrib/opensolaris/lib/libzfs/common/libzfs_import.c
===================================================================
--- cddl/contrib/opensolaris/lib/libzfs/common/libzfs_import.c	(revision 218622)
+++ cddl/contrib/opensolaris/lib/libzfs/common/libzfs_import.c	(working copy)
@@ -869,7 +869,6 @@
 	 * and toplevel GUID.
 	 */
 	for (i = 0; i < argc; i++) {
-		char *rdsk;
 		int dfd;
 
 		/* use realpath to normalize the path */
@@ -889,21 +888,11 @@
 			continue;
 		}
 
-		/*
-		 * Using raw devices instead of block devices when we're
-		 * reading the labels skips a bunch of slow operations during
-		 * close(2) processing, so we replace /dev/dsk with /dev/rdsk.
-		 */
-		if (strcmp(path, "/dev/dsk/") == 0)
-			rdsk = "/dev/rdsk/";
-		else
-			rdsk = path;
-
-		if ((dirp = opendir(rdsk)) == NULL) {
+		if ((dirp = opendir(path)) == NULL) {
 			zfs_error_aux(hdl, strerror(errno));
 			(void) zfs_error_fmt(hdl, EZFS_BADPATH,
 			    dgettext(TEXT_DOMAIN, "cannot open '%s'"),
-			    rdsk);
+			    path);
 			goto error;
 		}
 
@@ -916,8 +905,7 @@
 			    (name[1] == 0 || (name[1] == '.' && name[2] == 0)))
 				continue;
 
-			(void) snprintf(path, sizeof (path), "%s/%s", rdsk,
-			    dp->d_name);
+			(void) strlcpy(end, name, pathleft);
 
 			if ((fd = open64(path, O_RDONLY)) < 0)
 				continue;
@@ -965,8 +953,7 @@
 					config = NULL;
 					continue;
 				}
-				/* use the non-raw path for the config */
-				(void) strlcpy(end, name, pathleft);
+
 				if (add_config(hdl, &pools, path, config) != 0)
 					goto error;
 			}
--- zpool-import-d-patch ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list