[vbox-dev] [PATCH] misc fixes for Solaris guest shared folders

Life is hard, and then you die ronald at innovation.ch
Thu Jan 13 11:09:39 GMT 2011


Attached are some fixes to the shared folders on Solaris guests. The
first 3 are really minor, but the last two fix some nasty issues
around seeking during readdir (these were mostly introduced by my
fixes and enhancements in this area last summer :-( ).

If you're maintaining a 3.2 branch and planning on further releases
there, then I would recommend applying these fixes to that branch too
(I believe they should apply cleanly there too).

These patches are under MIT license and/or public-domain.


  Cheers,

  Ronald

-------------- next part --------------

Fix parameter to panic() call.

diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
index c932111..7975b10 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
@@ -304,7 +304,8 @@ top:
 	if (parent != NULL) {
 		sfnode_clear_dir_list(parent);
 		if (parent->sf_children == 0)
-			panic("sfnode_destroy(%s) parent has no child", node->sf_path);
+			panic("sfnode_destroy parent(%s) has no child",
+			    parent->sf_path);
 		--parent->sf_children;
 		if (parent->sf_children == 0 &&
 		    parent->sf_is_stale &&
-------------- next part --------------

Fixed compiler warning.

diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
index 7975b10..fb28a75 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
@@ -1893,7 +1893,7 @@ sfnode_print(sfnode_t *node)
 }
 
 void
-sfnode_list()
+sfnode_list(void)
 {
 	sfnode_t *n;
 	for (n = avl_first(&sfnodes); n != NULL; n = AVL_NEXT(&sfnodes, n))

-------------- next part --------------

Always show current stat-ttl in mount options, even when default value
is being used.

diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vfs.c b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vfs.c
index ca5b4d2..a72cb60 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vfs.c
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vfs.c
@@ -308,6 +308,9 @@ sffs_mount(vfs_t *vfsp, vnode_t *mvp, struct mounta *uap, cred_t *cr)
 	    ddi_strtol(optval, NULL, 10, &val) == 0 &&
 	    (int)val == val)
 		stat_ttl = val;
+        else
+		vfs_setmntopt(vfsp, "stat_ttl", VBOXSOLQUOTE(DEF_STAT_TTL_MS),
+		    0);
 
 	/*
 	 * whether to honor fsync

-------------- next part --------------

Fix directory seeking bug. The d_off field in the returned dirent's was
wrong, so seek'ing to those would result in an invalid seek. This was
causing commands like 'du' to not work properly.

Also added better seek-bounds checking for potential seeks.

diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.c b/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.c
index 85c27bf..9beae72 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.c
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.c
@@ -874,12 +874,12 @@ sfprov_readdir(
 	uint32_t numbytes;
 	uint32_t nents;
 	uint32_t size;
-	uint32_t cnt;
 	sffs_dirents_t *cur_buf;
 	sffs_stats_t *cur_stats;
 	struct dirent64 *dirent;
 	sffs_stat_t *stat;
 	unsigned short reclen;
+	off_t offset;
 
 	*dirents = NULL;
 	*stats = NULL;
@@ -933,7 +933,7 @@ sfprov_readdir(
 		goto done;
 	}
 
-	cnt = 0;
+	offset = 0;
 	for (;;) {
 		numbytes = infobuff_alloc;
 		error = vboxCallDirInfo(&vbox_client, &fp->map, fp->handle,
@@ -987,10 +987,11 @@ sfprov_readdir(
 			    (((char *) &cur_buf->sf_entries[0]) + cur_buf->sf_len);
 			strcpy(&dirent->d_name[0], info->name.String.utf8);
 			dirent->d_reclen = reclen;
-			dirent->d_off = cnt;
+
+			offset += reclen;
+			dirent->d_off = offset;
 
 			cur_buf->sf_len += reclen;
-			++cnt;
 
 			/* save the stats */
 			stat = &cur_stats->sf_stats[cur_stats->sf_num];
diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
index fb28a75..254f8bc 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
@@ -1718,6 +1718,23 @@ sffs_seek(vnode_t *v, offset_t o, offset_t *no, caller_context_t *ct)
 {
 	if (*no < 0 || *no > MAXOFFSET_T)
 		return (EINVAL);
+
+	if (v->v_type == VDIR) {
+		sffs_dirents_t *cur_buf = VN2SFN(v)->sf_dir_list;
+		off_t offset = 0;
+
+		if (cur_buf == NULL)
+			return (0);
+
+		while (cur_buf != NULL) {
+			if (*no >= offset && *no <= offset + cur_buf->sf_len)
+				return (0);
+			offset += cur_buf->sf_len;
+			cur_buf = cur_buf->sf_next;
+		}
+		return (EINVAL);
+	}
+
 	return (0);
 }
 

-------------- next part --------------

Fix nasty bug when reading large directories. The dirent's and stats were
located in two separate, parallel buffers; but when calling readdir with
a non-zero offset we were skipping through only the dirent's, thereby ending
up associating the wrong stats with these dirents.

This fix merges the two buffers into one structure, thereby both eliminating
the possibility of this bug and also simplifying the code.

Also included is proper validation of the offset that is passed in to
readdir.

diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.c b/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.c
index 9beae72..7e12d9b 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.c
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.c
@@ -860,8 +860,7 @@ int
 sfprov_readdir(
 	sfp_mount_t *mnt,
 	char *path,
-	sffs_dirents_t **dirents,
-	sffs_stats_t **stats)
+	sffs_dirents_t **dirents)
 {
 	int error;
 	char *cp;
@@ -875,21 +874,20 @@ sfprov_readdir(
 	uint32_t nents;
 	uint32_t size;
 	sffs_dirents_t *cur_buf;
-	sffs_stats_t *cur_stats;
-	struct dirent64 *dirent;
+	struct sffs_dirent *dirent;
 	sffs_stat_t *stat;
 	unsigned short reclen;
+	unsigned short entlen;
 	off_t offset;
 
 	*dirents = NULL;
-	*stats = NULL;
 
 	error = sfprov_open(mnt, path, &fp);
 	if (error != 0)
 		return (ENOENT);
 
 	/*
-	 * Allocate the first dirents and stats buffers.
+	 * Allocate the first dirents buffers.
 	 */
 	*dirents = kmem_alloc(SFFS_DIRENTS_SIZE, KM_SLEEP);
 	if (*dirents == NULL) {
@@ -900,15 +898,6 @@ sfprov_readdir(
 	cur_buf->sf_next = NULL;
 	cur_buf->sf_len = 0;
 
-	*stats = kmem_alloc(sizeof(**stats), KM_SLEEP);
-	if (*stats == NULL) {
-		error = (ENOSPC);
-		goto done;
-	}
-	cur_stats = *stats;
-	cur_stats->sf_next = NULL;
-	cur_stats->sf_num = 0;
-
 	/*
 	 * Create mask that VBox expects. This needs to be the directory path,
 	 * plus a "*" wildcard to get all files.
@@ -960,7 +949,8 @@ sfprov_readdir(
 		for (info = infobuff; (char *) info < (char *) infobuff + numbytes; nents--) {
 			/* expand buffers if we need more space */
 			reclen = DIRENT64_RECLEN(strlen(info->name.String.utf8));
-			if (SFFS_DIRENTS_OFF + cur_buf->sf_len + reclen > SFFS_DIRENTS_SIZE) {
+			entlen = sizeof(sffs_stat_t) + reclen;
+			if (SFFS_DIRENTS_OFF + cur_buf->sf_len + entlen > SFFS_DIRENTS_SIZE) {
 				cur_buf->sf_next = kmem_alloc(SFFS_DIRENTS_SIZE, KM_SLEEP);
 				if (cur_buf->sf_next == NULL) {
 					error = ENOSPC;
@@ -971,31 +961,17 @@ sfprov_readdir(
 				cur_buf->sf_len = 0;
 			}
 
-			if (cur_stats->sf_num >= SFFS_STATS_LEN) {
-				cur_stats->sf_next = kmem_alloc(sizeof(**stats), KM_SLEEP);
-				if (cur_stats->sf_next == NULL) {
-					error = (ENOSPC);
-					goto done;
-				}
-				cur_stats = cur_stats->sf_next;
-				cur_stats->sf_next = NULL;
-				cur_stats->sf_num = 0;
-			}
-
 			/* create the dirent with the name, offset, and len */
-			dirent = (dirent64_t *)
+			dirent = (struct sffs_dirent *)
 			    (((char *) &cur_buf->sf_entries[0]) + cur_buf->sf_len);
-			strcpy(&dirent->d_name[0], info->name.String.utf8);
-			dirent->d_reclen = reclen;
-
-			offset += reclen;
-			dirent->d_off = offset;
+			strcpy(&dirent->sf_entry.d_name[0], info->name.String.utf8);
+			dirent->sf_entry.d_reclen = reclen;
 
-			cur_buf->sf_len += reclen;
+			offset += entlen;
+			dirent->sf_entry.d_off = offset;
 
 			/* save the stats */
-			stat = &cur_stats->sf_stats[cur_stats->sf_num];
-			++cur_stats->sf_num;
+			stat = &dirent->sf_stat;
 
 			sfprov_mode_from_fmode(&stat->sf_mode, info->Info.Attr.fMode);
 			stat->sf_size = info->Info.cbObject;
@@ -1004,6 +980,8 @@ sfprov_readdir(
 			sfprov_ftime_from_timespec(&stat->sf_ctime, &info->Info.ChangeTime);
 
 			/* next info */
+			cur_buf->sf_len += entlen;
+
 			size = offsetof (SHFLDIRINFO, name.String) + info->name.u16Size;
 			info = (SHFLDIRINFO *) ((uintptr_t) info + size);
 		}
@@ -1022,11 +1000,6 @@ done:
 			kmem_free(*dirents, SFFS_DIRENTS_SIZE);
 			*dirents = cur_buf;
 		}
-		while (*stats) {
-			cur_stats = (*stats)->sf_next;
-			kmem_free(*stats, sizeof(**stats));
-			*stats = cur_stats;
-		}
 	}
 	if (infobuff != NULL)
 		kmem_free(infobuff, infobuff_alloc);
diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.h b/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.h
index 1fd1fa0..c04f1f9 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.h
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_prov.h
@@ -123,19 +123,9 @@ extern int sfprov_rename(sfp_mount_t *, char *from, char *to, uint_t is_dir);
  * Read directory entries.
  */
 /*
- * a singly linked list of buffers, each containing an array of dirent's.
+ * a singly linked list of buffers, each containing an array of stat's+dirent's.
  * sf_len is length of the sf_entries array, in bytes.
  */
-typedef struct sffs_dirents {
-	struct sffs_dirents	*sf_next;
-	len_t			sf_len;
-	dirent64_t		sf_entries[1];
-} sffs_dirents_t;
-
-#define SFFS_DIRENTS_SIZE	8192
-#define SFFS_DIRENTS_OFF	(offsetof(sffs_dirents_t, sf_entries[0]))
-#define SFFS_STATS_LEN		100
-
 typedef struct sffs_stat {
 	mode_t		sf_mode;
 	off_t		sf_size;
@@ -144,14 +134,20 @@ typedef struct sffs_stat {
 	timestruc_t	sf_ctime;
 } sffs_stat_t;
 
-typedef struct sffs_stats {
-	struct sffs_stats	*sf_next;
-	len_t			sf_num;
-	sffs_stat_t		sf_stats[SFFS_STATS_LEN];
-} sffs_stats_t;
+typedef struct sffs_dirents {
+	struct sffs_dirents	*sf_next;
+	len_t			sf_len;
+	struct sffs_dirent {
+		sffs_stat_t	sf_stat;
+		dirent64_t	sf_entry;	/* this is variable length */
+	}			sf_entries[1];
+} sffs_dirents_t;
+
+#define SFFS_DIRENTS_SIZE	8192
+#define SFFS_DIRENTS_OFF	(offsetof(sffs_dirents_t, sf_entries[0]))
 
-extern int sfprov_readdir(sfp_mount_t *mnt, char *path, sffs_dirents_t **dirents,
-    sffs_stats_t **stats);
+extern int sfprov_readdir(sfp_mount_t *mnt, char *path,
+    sffs_dirents_t **dirents);
 
 #ifdef	__cplusplus
 }
diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
index 254f8bc..d54f3c3 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.c
@@ -165,12 +165,6 @@ sfnode_clear_dir_list(sfnode_t *node)
 		kmem_free(node->sf_dir_list, SFFS_DIRENTS_SIZE);
 		node->sf_dir_list = next;
 	}
-
-	while (node->sf_dir_stats != NULL) {
-		sffs_stats_t *next = node->sf_dir_stats->sf_next;
-		kmem_free(node->sf_dir_stats, sizeof(*node->sf_dir_stats));
-		node->sf_dir_stats = next;
-	}
 }
 
 /*
@@ -253,7 +247,6 @@ sfnode_make(
 	if (parent)
 		++parent->sf_children;
 	node->sf_dir_list = NULL;
-	node->sf_dir_stats = NULL;
 	if (stat != NULL) {
 		node->sf_stat = *stat;
 		node->sf_stat_time = stat_time;
@@ -694,11 +687,10 @@ sffs_readdir(
 {
 	sfnode_t *dir = VN2SFN(vp);
 	sfnode_t *node;
-	struct dirent64 *dirent;
+	struct sffs_dirent *dirent = NULL;
 	sffs_dirents_t *cur_buf;
-	sffs_stats_t *cur_stats;
-	int cur_snum;
-	offset_t offset;
+	offset_t offset = 0;
+	offset_t orig_off = uiop->uio_loffset;
 	int dummy_eof;
 	int error = 0;
 
@@ -726,61 +718,92 @@ sffs_readdir(
 
 	if (dir->sf_dir_list == NULL) {
 		error = sfprov_readdir(dir->sf_sffs->sf_handle, dir->sf_path,
-		    &dir->sf_dir_list, &dir->sf_dir_stats);
+		    &dir->sf_dir_list);
 		if (error != 0)
 			goto done;
 	}
 
 	/*
+	 * Validate and skip to the desired offset.
+	 */
+	cur_buf = dir->sf_dir_list;
+	offset = 0;
+
+	while (cur_buf != NULL &&
+	    offset + cur_buf->sf_len <= uiop->uio_loffset) {
+		offset += cur_buf->sf_len;
+		cur_buf = cur_buf->sf_next;
+	}
+
+	if (cur_buf == NULL && offset != uiop->uio_loffset) {
+		error = EINVAL;
+		goto done;
+	}
+	if (cur_buf != NULL && offset != uiop->uio_loffset) {
+		offset_t off = offset;
+		int step;
+		dirent = &cur_buf->sf_entries[0];
+
+		while (off < uiop->uio_loffset) {
+			if (dirent->sf_entry.d_off == uiop->uio_loffset)
+				break;
+			step = sizeof(sffs_stat_t) + dirent->sf_entry.d_reclen;
+			dirent = (struct sffs_dirent *) (((char *) dirent) + step);
+			off += step;
+		}
+
+		if (off >= uiop->uio_loffset) {
+			error = EINVAL;
+			goto done;
+		}
+	}
+
+	offset = uiop->uio_loffset - offset;
+
+	/*
 	 * Lookup each of the names, so that we have ino's, and copy to
 	 * result buffer.
 	 */
-	offset = 0;
-	cur_buf = dir->sf_dir_list;
-	cur_stats = dir->sf_dir_stats;
-	cur_snum = 0;
 	while (cur_buf != NULL) {
-		if (offset + cur_buf->sf_len <= uiop->uio_loffset) {
-			offset += cur_buf->sf_len;
+		if (offset >= cur_buf->sf_len) {
 			cur_buf = cur_buf->sf_next;
+			offset  = 0;
 			continue;
 		}
 
-		if (cur_snum >= SFFS_STATS_LEN) {
-			cur_stats = cur_stats->sf_next;
-			cur_snum = 0;
-		}
-
-		dirent = (dirent64_t *)
-		    (((char *) &cur_buf->sf_entries[0]) +
-		     (uiop->uio_loffset - offset));
-		if (dirent->d_reclen > uiop->uio_resid)
+		dirent = (struct sffs_dirent *)
+		    (((char *) &cur_buf->sf_entries[0]) + offset);
+		if (dirent->sf_entry.d_reclen > uiop->uio_resid)
 			break;
 
-		if (strcmp(dirent->d_name, ".") == 0) {
+		if (strcmp(dirent->sf_entry.d_name, ".") == 0) {
 			node = dir;
-		} else if (strcmp(dirent->d_name, "..") == 0) {
+		} else if (strcmp(dirent->sf_entry.d_name, "..") == 0) {
 			node = dir->sf_parent;
 			if (node == NULL)
 				node = dir;
 		} else {
-			node = sfnode_lookup(dir, dirent->d_name, VNON,
-			    &cur_stats->sf_stats[cur_snum],
-			    sfnode_cur_time_usec(), NULL);
+			node = sfnode_lookup(dir, dirent->sf_entry.d_name, VNON,
+			    &dirent->sf_stat, sfnode_cur_time_usec(), NULL);
 			if (node == NULL)
 				panic("sffs_readdir() lookup failed");
 		}
-		dirent->d_ino = node->sf_ino;
+		dirent->sf_entry.d_ino = node->sf_ino;
 
-		error = uiomove(dirent, dirent->d_reclen, UIO_READ, uiop);
-		++cur_snum;
+		error = uiomove(&dirent->sf_entry, dirent->sf_entry.d_reclen,
+		    UIO_READ, uiop);
 		if (error != 0)
 			break;
+
+		uiop->uio_loffset = dirent->sf_entry.d_off;
+		offset += sizeof(sffs_stat_t) + dirent->sf_entry.d_reclen;
 	}
 	if (error == 0 && cur_buf == NULL)
 		*eofp = 1;
 done:
 	mutex_exit(&sffs_lock);
+	if (error != 0)
+		uiop->uio_loffset = orig_off;
 	return (error);
 }
 
diff --git a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.h b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.h
index 5da140d..456b948 100644
--- a/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.h
+++ b/src/VBox/Additions/solaris/SharedFolders/vboxfs_vnode.h
@@ -59,7 +59,6 @@ typedef struct sfnode {
 	sffs_stat_t	sf_stat;	/* cached file attrs for this node */
 	uint64_t	sf_stat_time;	/* last-modified time of sf_stat */
 	sffs_dirents_t	*sf_dir_list;	/* list of entries for this directory */
-	sffs_stats_t	*sf_dir_stats;	/* file attrs for the above entries */
 } sfnode_t;
 
 #define VN2SFN(vp) ((sfnode_t *)(vp)->v_data)



More information about the vbox-dev mailing list