Patchwork [6/7] libjunction: Buffer overrun in fedfs_get_xattr()

login
register
mail settings
Submitter Chuck Lever
Date Nov. 3, 2011, 3:29 p.m.
Message ID <20111103152904.2445.63715.stgit@degas.1015granger.net>
Download mbox | patch
Permalink /patch/123460/
State Accepted
Headers show

Comments

Chuck Lever - Nov. 3, 2011, 3:29 p.m.
Callers of fedfs_get_xattr() assume the returned buffer is NUL-
terminated.  It's not: it's an opaque stream of bytes.

No caller expects an opaque byte stream, though.  So convert
fedfs_get_xattr() to return a NUL-terminated C string in a fresh
buffer.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 src/libjunction/junction.c |   45 ++++++++++++++++++++++----------------------
 1 files changed, 23 insertions(+), 22 deletions(-)

Patch

diff --git a/src/libjunction/junction.c b/src/libjunction/junction.c
index 6bc5310..73aa779 100644
--- a/src/libjunction/junction.c
+++ b/src/libjunction/junction.c
@@ -239,22 +239,22 @@  fedfs_is_xattr_present(int fd, const char *path, const char *name)
 }
 
 /**
- * Retrieve the contents of xattr "name"
+ * Read the contents of xattr "name"
  *
  * @param fd an open file descriptor
  * @param path NUL-terminated C string containing pathname of a directory
  * @param name NUL-terminated C string containing name of xattr to retrieve
- * @param contents OUT: opaque byte array containing contents of xattr
- * @param contentlen OUT: size of "contents"
+ * @param contents OUT: NUL-terminated C string containing contents of xattr
  * @return a FedFsStatus code
  *
+ * Caller must free string returned in "contents" with free(3).
+ *
  * @note Access to trusted attributes requires CAP_SYS_ADMIN.
  */
 static FedFsStatus
-fedfs_get_xattr(int fd, const char *path, const char *name, void **contents,
-		size_t *contentlen)
+fedfs_read_xattr(int fd, const char *path, const char *name, char **contents)
 {
-	void *xattrbuf = NULL;
+	char *xattrbuf = NULL;
 	ssize_t len;
 
 	len = fgetxattr(fd, name, xattrbuf, 0);
@@ -264,7 +264,7 @@  fedfs_get_xattr(int fd, const char *path, const char *name, void **contents,
 		return FEDFS_ERR_ACCESS;
 	}
 
-	xattrbuf = malloc(len);
+	xattrbuf = (char *)malloc(len + 1);
 	if (xattrbuf == NULL) {
 		xlog(D_GENERAL, "%s: failed to get buffer for xattr %s on %s",
 			__func__, name, path);
@@ -277,11 +277,11 @@  fedfs_get_xattr(int fd, const char *path, const char *name, void **contents,
 		free(xattrbuf);
 		return FEDFS_ERR_ACCESS;
 	}
+	xattrbuf[len] = '\0';
 
 	xlog(D_CALL, "%s: read xattr %s from path %s",
 			__func__, name, path);
 	*contents = xattrbuf;
-	*contentlen = len;
 	return FEDFS_OK;
 }
 
@@ -440,37 +440,39 @@  out_err:
  * @param fsn_uuid OUT: NUL-terminated C string containing FSN UUID to store
  * @param host OUT: an initialized nsdb_t object
  * @return a FedFsStatus code
+ *
+ * Caller must free string returned in "fsn_uuid" with free(3), and the
+ * host returned in "host" with nsdb_free_nsdb().
  */
 FedFsStatus
 fedfs_get_fsn(const char *pathname, char **fsn_uuid, nsdb_t *host)
 {
-	void *uuid_tmp = NULL;
-	void *nsdbname_tmp = NULL;
-	void *port_tmp = NULL;
+	char *uuid_tmp = NULL;
+	char *nsdbname_tmp = NULL;
+	char *port_tmp = NULL;
 	nsdb_t host_tmp = NULL;
 	unsigned short port;
 	FedFsStatus retval;
-	size_t len;
 	int fd;
 
-	if (fsn_uuid == NULL || host == NULL)
+	if (pathname == NULL || fsn_uuid == NULL || host == NULL)
 		return FEDFS_ERR_INVAL;
 
 	retval = fedfs_open_path(pathname, &fd);
 	if (retval != FEDFS_OK)
 		return retval;
 
-	retval = fedfs_get_xattr(fd, pathname, FEDFSD_XATTR_NAME_FSNUUID,
-							&uuid_tmp, &len);
+	retval = fedfs_read_xattr(fd, pathname, FEDFSD_XATTR_NAME_FSNUUID,
+								&uuid_tmp);
 	if (retval != FEDFS_OK)
 		goto out_err;
 
-	retval = fedfs_get_xattr(fd, pathname, FEDFSD_XATTR_NAME_NSDB,
-					&nsdbname_tmp, &len);
+	retval = fedfs_read_xattr(fd, pathname, FEDFSD_XATTR_NAME_NSDB,
+								&nsdbname_tmp);
 	if (retval != FEDFS_OK)
 		goto out_err;
-	retval = fedfs_get_xattr(fd, pathname, FEDFSD_XATTR_NAME_PORT,
-					&port_tmp, &len);
+	retval = fedfs_read_xattr(fd, pathname, FEDFSD_XATTR_NAME_PORT,
+								&port_tmp);
 	if (retval != FEDFS_OK)
 		goto out_err;
 
@@ -670,15 +672,14 @@  fedfs_restore_mode(const char *pathname)
 {
 	FedFsStatus retval;
 	mode_t mode;
-	size_t len;
-	void *buf;
+	char *buf;
 	int fd;
 
 	retval = fedfs_open_path(pathname, &fd);
 	if (retval != FEDFS_OK)
 		return retval;
 
-	retval = fedfs_get_xattr(fd, pathname, FEDFSD_XATTR_NAME_MODE, &buf, &len);
+	retval = fedfs_read_xattr(fd, pathname, FEDFSD_XATTR_NAME_MODE, &buf);
 	if (retval != FEDFS_OK)
 		goto out;