PERFORCE change 166062 for review

Jonathan Anderson jona at FreeBSD.org
Tue Jul 14 06:40:51 UTC 2009


http://perforce.freebsd.org/chv.cgi?CH=166062

Change 166062 by jona at jona-trustedbsd-belle-vmware on 2009/07/14 06:40:41

	Much more sensible error handling for libuserangel

Affected files ...

.. //depot/projects/trustedbsd/capabilities/src/lib/libuserangel/libuserangel.c#8 edit
.. //depot/projects/trustedbsd/capabilities/src/lib/libuserangel/libuserangel.h#10 edit

Differences ...

==== //depot/projects/trustedbsd/capabilities/src/lib/libuserangel/libuserangel.c#8 (text+ko) ====

@@ -39,6 +39,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -57,18 +58,14 @@
 const char *VERSION = "UA/0.1";
 const char *ua_protocol_version() { return VERSION; }
 
-char errmsg[256];
-const char* ua_protocol_error(void) { return errmsg; }
 
-
 int ua_find(void)
 {
 	char *homedir = getenv("HOME");
 
 	if(strlen(homedir) > 200)
 	{
-		sprintf(errmsg, "Obscenely long $HOME variable (%i chars)",
-		        strlen(homedir));
+		errno = ENAMETOOLONG;
 		return -1;
 	}
 
@@ -81,27 +78,20 @@
 	strcpy(addr.sun_path, control_socket_name);
 
 	angel = socket(AF_UNIX, SOCK_STREAM, 0);
-	if(connect(angel, (struct sockaddr*) &addr, sizeof(addr)))
-	{
-		sprintf(errmsg, "Error connecting to angel at '%s'", addr.sun_path);
-		return -1;
-	}
+	if(connect(angel, (struct sockaddr*) &addr, sizeof(addr))) return -1;
 
-	if(lc_limitfd(angel, CAP_READ | CAP_WRITE) < 0)
-	{
-		sprintf(errmsg, "Error creating user angel capability: %i (%s)",
-		        errno, strerror(errno));
-		return -1;
-	}
+	int on = 1;
+	if(setsockopt(angel, SOL_SOCKET, SO_NOSIGPIPE, &on, sizeof(on))) return -1;
+	if(lc_limitfd(angel, CAP_READ | CAP_WRITE) < 0) return -1;
 
 
 	// receive server 'hello'
 	struct ua_datum *hello_datum = ua_recv(angel, NULL, NULL);
 	if(!hello_datum)
 	{
-		sprintf(errmsg, "Error receiving server 'hello': %i (%s)",
-		        errno, strerror(errno));
+		int olderrno = errno;
 		close(angel);
+		errno = olderrno;
 		angel = -1;
 		return -1;
 	}
@@ -110,10 +100,9 @@
 	char hello[hellolen];
 	if(ua_unmarshall_string(hello_datum, hello, &hellolen) < 0)
 	{
-		sprintf(errmsg, "Error unmarshalling server 'hello': %i (%s)",
-		        errno, strerror(errno));
-
+		int olderrno = errno;
 		close(angel);
+		errno = olderrno;
 		angel = -1;
 		return -1;
 	}
@@ -122,10 +111,8 @@
 	// validate server 'hello' message
 	if(strncmp(hello, "user_angel", 10))
 	{
-		sprintf(errmsg, "Server handshake didn't start with user_angel: %s",
-		        hello);
-
 		close(angel);
+		errno = EFTYPE;
 		angel = -1;
 		return -1;
 	}
@@ -137,10 +124,8 @@
 	unsigned int len = strlen(version);
 	if((len != strlen(VERSION)) || strncmp(version, VERSION, len))
 	{
-		sprintf(errmsg, "Invalid UA protocol version: %s (expected %s)",
-		        version, VERSION);
-
 		close(angel);
+		errno = ERPCMISMATCH;
 		angel = -1;
 		return -1;
 	}
@@ -156,17 +141,24 @@
 
 int ua_open(const char *path, int flags)
 {
-	if(angel < 0) angel = ua_find();
-	if(angel < 0) return -1;
-
 	cap_rights_t rights = CAP_SEEK | CAP_FSYNC;
 
 	if(flags & O_WRONLY) rights |= CAP_WRITE | CAP_FTRUNCATE;
 	else if(flags & O_RDWR) rights |= CAP_READ | CAP_WRITE | CAP_FTRUNCATE;
 	else rights |= CAP_READ;
 
-	if(flags & O_EXEC) rights |= CAP_FEXECVE;
+	if(flags & O_EXEC) rights |= CAP_FSTAT | CAP_FEXECVE;
+
+	return ua_ropen(path, flags, rights);
+}
+
+
+int ua_ropen(const char *path, int flags, cap_rights_t rights)
+{
+	if(angel < 0) angel = ua_find();
+	if(angel < 0) return -1;
 
+	printf("ua_ropen('%s', %i, %016llx)\n", path, flags, rights);
 
 	struct ua_datum *data[4];
 	data[0] = ua_marshall_int(UA_OPEN_PATH);
@@ -175,13 +167,7 @@
 	data[3] = ua_marshall_int(rights);
 
 
-	for(int i = 0; i < 4; i++)
-		if(ua_send(angel, data[i], NULL, 0) < 0)
-		{
-			sprintf(errmsg, "Error sending request message: %s",
-		        ua_protocol_error());
-			return -1;
-		}
+	for(int i = 0; i < 4; i++) if(ua_send(angel, data[i], NULL, 0) < 0) return -1;
 
 	free(data[0]);
 	free(data[1]);
@@ -192,45 +178,25 @@
 
 	// retrieve the file descriptor(s)
 	struct ua_datum *fdcountd = ua_recv(angel, NULL, NULL);
-	if(!fdcountd)
-	{
-		sprintf(errmsg, "Error receiving FD count: %s",
-		        ua_protocol_error());
-		return -1;
-	}
+	if(!fdcountd) return -1;
 
 	int fdcount;
-	if(ua_unmarshall_int(fdcountd, &fdcount) < 0)
-	{
-		fprintf(stderr, "Error unmarshalling FD count: %s\n",
-		                 ua_protocol_error());
-		return -1;
-	}
+	if(ua_unmarshall_int(fdcountd, &fdcount) < 0) return -1;
 
 	if(fdcount != 1)
 	{
-		sprintf(errmsg, "Receiving %i FDs, only asked for 1", fdcount);
+		errno = EOVERFLOW;
 		return -1;
 	}
 
 	int32_t fd;
 	unsigned int fdlen = 1;
 	struct ua_datum *fd_datum = ua_recv(angel, &fd, &fdlen);
-	if(!fd_datum)
-	{
-		sprintf(errmsg, "Error receiving FD: %s",
-		        ua_protocol_error());
-		return -1;
-	}
+	if(!fd_datum) return -1;
 
 	unsigned int namelen = 80;
 	char name[namelen];
-	if(ua_unmarshall_string(fd_datum, name, &namelen) < 0)
-	{
-		sprintf(errmsg, "Error unmarshalling FD name: %s",
-		        ua_protocol_error());
-		return -1;
-	}
+	if(ua_unmarshall_string(fd_datum, name, &namelen) < 0) return -1;
 
 	return fd;
 }
@@ -280,9 +246,9 @@
 	if(flags & O_APPEND)
 		if(lseek(fd, 0, SEEK_END) < 0)
 		{
-			sprintf(errmsg, "Error seeking to end of %s: %i (%s)",
-			        path, errno, strerror(errno));
+			int olderrno = errno;
 			close(fd);
+			errno = olderrno;
 			return NULL;
 		}
 
@@ -305,12 +271,7 @@
 	{
 		int cmsghdrlen = sizeof(struct cmsghdr) + fdlen * sizeof(int32_t);
 		anc_hdr = (struct cmsghdr*) malloc(cmsghdrlen);
-		if(!anc_hdr)
-		{
-			sprintf(errmsg, "Error creating ancilliary header: %i (%s)",
-			        errno, strerror(errno));
-			return -1;
-		}
+		if(!anc_hdr) return -1;
 
 		anc_hdr->cmsg_len = cmsghdrlen;
 		anc_hdr->cmsg_level = SOL_SOCKET;
@@ -334,13 +295,7 @@
 
 	// send!
 	int bytes_sent = sendmsg(sock, &header, 0);
-	if(bytes_sent < 0)
-	{
-		sprintf(errmsg, "Error sending data and file descriptors: %i (%s)",
-                                 errno, strerror(errno));
-		free(anc_hdr);
-		return -1;
-	}
+	if(bytes_sent < 0) return -1;
 
 
 	free(anc_hdr);
@@ -353,18 +308,7 @@
 	// how much data is there to receive?
 	datum peek;
 	int bytes = recv(sock, &peek, sizeof(datum), MSG_PEEK); 
-
-	if(bytes == 0)
-	{
-		sprintf(errmsg, "Socket closed: %s", strerror(errno));
-		return NULL;
-	}
-	else if(bytes < 0)
-	{
-		sprintf(errmsg, "Error peeking at socket: %i (%s)",
-		                 errno, strerror(errno));
-		return NULL;
-	}
+	if(bytes <= 0) return NULL;
 
 	int to_receive = sizeof(datum) + peek.length;
 
@@ -382,11 +326,7 @@
 	struct cmsghdr *anc_hdr = (struct cmsghdr*) malloc(size);
 	bzero(anc_hdr, size);
 
-	if(!anc_hdr)
-	{
-		sprintf(errmsg, "Error creating ancilliary data header: %i (%s)",
-		        errno, strerror(errno));
-	}
+	if(!anc_hdr) return NULL;
 	anc_hdr->cmsg_len = size;
 	anc_hdr->cmsg_level = SOL_SOCKET;
 	anc_hdr->cmsg_type = (maxfds ? SCM_RIGHTS : 0);
@@ -404,21 +344,14 @@
 
 
 	bytes = recvmsg(sock, &header, MSG_WAITALL);
-	if(bytes < 0)
+	if(bytes <= 0)
 	{
-		sprintf(errmsg, "Error receiving message: %i (%s)",
-		                errno, strerror(errno));
+		int olderrno = errno;
 		free(anc_hdr);
 		free(d);
+		errno = olderrno;
 		return NULL;
 	}
-	else if(bytes == 0)
-	{
-		sprintf(errmsg, "Socket closed: %i (%s)", errno, strerror(errno));
-		free(anc_hdr);
-		free(d);
-		return NULL;
-	}
 
 
 	if(maxfds > 0)
@@ -461,24 +394,23 @@
 {
 	if(d == NULL)
 	{
-		sprintf(errmsg, "Datum is NULL");
+		errno = EINVAL;
 		return -1;
 	}
 
 	if(!(d->type & INTEGER))
 	{
 		if(d->type & ERROR) handle_error(d);
-		else sprintf(errmsg, "Datum's type is %i, not INTEGER (%i)",
-		             d->type, INTEGER);
+		else errno = EINVAL;
 		return -1;
 	}
 
-	if(d->length != sizeof(int32_t))
+	if(d->length > sizeof(int32_t))
 	{
-		sprintf(errmsg, "An 32-bit integer should be 4B long, not %i",
-		        d->length);
+		errno = EOVERFLOW;
 		return -1;
 	}
+	else bzero(value, sizeof(int32_t));
 
 	memcpy(value, ((const char*) d) + sizeof(datum), 4);
 	return d->length;
@@ -504,19 +436,21 @@
 {
 	if(d == NULL)
 	{
-		sprintf(errmsg, "NULL datum");
+		errno = EINVAL;
 		return -1;
 	}
 	else if(d->type != STRING)
 	{
 		if(d->type & ERROR) handle_error(d);
-		else sprintf(errmsg, "Datum's type is %i, not STRING (%i)",
-		                     d->type, STRING);
+		else
+		{
+			errno = EINVAL;
+			return -1;
+		}
 	}
 	else if(d->length >= *len)
 	{
-		sprintf(errmsg, "String in datum is too long (%iB) to fit in buffer (%iB)",
-		        d->length, *len);
+		errno = EOVERFLOW;
 		return -1;
 	}
 
@@ -552,18 +486,17 @@
 {
 	if(d == NULL)
 	{
-		sprintf(errmsg, "NULL datum");
+		errno = EINVAL;
 		return -1;
 	}
 	else if(d->type != ERROR)
 	{
-		sprintf(errmsg, "Datum's type is %i, not ERROR (%i)",
-		        d->type, ERROR);
+		errno = EINVAL;
+		return -1;
 	}
 	else if(d->length >= (*msglen - sizeof(int32_t)))
 	{
-		sprintf(errmsg, "Message is too long (%iB) to fit in buffer (%iB)",
-		        d->length - sizeof(int32_t), *msglen);
+		errno = EOVERFLOW;
 		return -1;
 	}
 
@@ -596,7 +529,7 @@
 	for(int i = 0; i < 8; i++)
 		if(data[i] == NULL)
 		{
-			sprintf(errmsg, "Capbox datum %i is NULL", i);
+			errno = EINVAL;
 			return NULL;
 		}
 		else total_size += (sizeof(datum) + data[i]->length);
@@ -623,37 +556,24 @@
 {
 	if(d == NULL)
 	{
-		sprintf(errmsg, "NULL datum");
+		errno = EINVAL;
 		return -1;
 	}
 	else if(d->type != CAPBOX_OPTIONS)
 	{
 		if(d->type & ERROR) handle_error(d);
-		else sprintf(errmsg, "Datum's type is %i, not CAPBOX_OPTIONS (%i)",
-		                     d->type, CAPBOX_OPTIONS);
+		else errno = EINVAL;
 		return -1;
 	}
 
 	int32_t tmp_int;
 	const datum *head = (const datum*) (((const char*) d) + sizeof(datum));
+	if(ua_unmarshall_int(head, &tmp_int) < 0) return -1;
 
-	if(ua_unmarshall_int(head, &tmp_int) < 0)
-	{
-		char error[128];
-		sprintf(error, "Error unmarshalling UI type: %s", ua_protocol_error());
-		strcpy(errmsg, error);
-		return -1;
-	}
 	options->ui = tmp_int;
 	head = (const datum*) (((const char*) head) + sizeof(datum) + head->length);
+	if(ua_unmarshall_int(head, &tmp_int) < 0) return -1;
 
-	if(ua_unmarshall_int(head, &tmp_int) < 0)
-	{
-		char error[128];
-		sprintf(error, "Error unmarshalling operation: %s", ua_protocol_error());
-		strcpy(errmsg, error);
-		return -1;
-	}
 	options->operation = tmp_int;
 	head = (const datum*) (((const char*) head) + sizeof(datum) + head->length);
 
@@ -661,25 +581,13 @@
 	// window title is handled elsewhere
 
 
-	if(ua_unmarshall_int(head, &tmp_int) < 0)
-	{
-		char error[128];
-		sprintf(error, "Error unmarshalling parent: %s", ua_protocol_error());
-		strcpy(errmsg, error);
-		return -1;
-	}
+	if(ua_unmarshall_int(head, &tmp_int) < 0) return -1;
 	options->parent_window = tmp_int;
 	head = (const datum*) (((const char*) head) + sizeof(datum) + head->length);
 
 	options->pathlen = d->length + 1;
 	char *str = (char*) malloc(options->pathlen);
-	if(ua_unmarshall_string(head, str, &options->pathlen) < 0)
-	{
-		char error[128];
-		sprintf(error, "Error unmarshalling path: %s", ua_protocol_error());
-		strcpy(errmsg, error);
-		return -1;
-	}
+	if(ua_unmarshall_string(head, str, &options->pathlen) < 0) return -1;
 	options->start_path = str;
 	head = (const datum*) (((const char*) head) + sizeof(datum) + head->length);
 
@@ -688,45 +596,21 @@
 	// that's handled at the recvmsg() level
 
 
-	if(ua_unmarshall_int(head, &tmp_int) < 0)
-	{
-		char error[128];
-		sprintf(error, "Error unmarshalling 'mult': %s", ua_protocol_error());
-		strcpy(errmsg, error);
-		return -1;
-	}
+	if(ua_unmarshall_int(head, &tmp_int) < 0) return -1;
 	options->mult = tmp_int;
 	head = (const datum*) (((const char*) head) + sizeof(datum) + head->length);
 
 	options->filterlen = d->length + 1;
 	str = (char*) malloc(options->filterlen);
-	if(ua_unmarshall_string(head, str, &options->filterlen) < 0)
-	{
-		char error[128];
-		sprintf(error, "Error unmarshalling filter: %s", ua_protocol_error());
-		strcpy(errmsg, error);
-		return -1;
-	}
+	if(ua_unmarshall_string(head, str, &options->filterlen) < 0) return -1;
 	options->filter = str;
 	head = (const datum*) (((const char*) head) + sizeof(datum) + head->length);
 
-	if(ua_unmarshall_int(head, &tmp_int) < 0)
-	{
-		char error[128];
-		sprintf(error, "Error unmarshalling 'flags': %s", ua_protocol_error());
-		strcpy(errmsg, error);
-		return -1;
-	}
+	if(ua_unmarshall_int(head, &tmp_int) < 0) return -1;
 	options->flags = tmp_int;
 	head = (const datum*) (((const char*) head) + sizeof(datum) + head->length);
 
-	if(ua_unmarshall_int(head, &tmp_int) < 0)
-	{
-		char error[128];
-		sprintf(error, "Error unmarshalling 'rights': %s", ua_protocol_error());
-		strcpy(errmsg, error);
-		return -1;
-	}
+	if(ua_unmarshall_int(head, &tmp_int) < 0) return -1;
 	options->rights = tmp_int;
 
 
@@ -740,13 +624,13 @@
 	char msg[200];
 	int msglen = 200;
 
-	if(ua_unmarshall_error(d, &errnum, msg, &msglen) < 0)
-	{
-		sprintf(errmsg, "Error unmarshalling error message\n");
-		return;
-	}
+	if(ua_unmarshall_error(d, &errnum, msg, &msglen) < 0) return;
+
+	errno = errnum;
 
-	sprintf(errmsg, "user_angel error: %s (%i: %s)",
-	                msg, errnum, strerror(errnum));
+	/* TODO: do something!
+	fprintf(stderr, "user_angel error: %s (%i: %s)\n",
+	                 msg, errnum, strerror(errnum));
+	 */
 }
 

==== //depot/projects/trustedbsd/capabilities/src/lib/libuserangel/libuserangel.h#10 (text+ko) ====

@@ -47,9 +47,6 @@
 /** Version of the User Angel protocol */
 const char* ua_protocol_version(void);
 
-/** The last angel/sandbox protocol error */
-const char* ua_protocol_error(void);
-
 /** Find the user angel (at $HOME/.user-angel or the like) */
 int ua_find(void);
 
@@ -59,6 +56,9 @@
 /** Open a file via the User Angel */
 int ua_open(const char *path, int flags);
 
+/** Open a file via the User Angel, specifying rights the capability should have */
+int ua_ropen(const char *path, int flags, cap_rights_t rights);
+
 /** Open a file stream via the User Angel */
 FILE* ua_fopen(const char *path, const char *mode);
 


More information about the p4-projects mailing list