Use actual length of socket's name for abstract sockets (v3)

The addrlen parameter should be the actual length of socket's name for abstract sockets. Otherwise socket gets padded with NULLs.

cat /proc/net/unix | grep lxc
[...]
0000000000000000: 00000003 00000000 00000000 0001 03 226548 @lxc/ad055575fe28ddd5//var/lib/lxc^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@^@
[...]

with this patch;

cat /proc/net/unix | grep lxc
[...]
0000000000000000: 00000002 00000000 00010000 0001 01 109563 @lxc/ad055575fe28ddd5//var/lib/lxc
[...]

Changes since v1:
    * check the length of passed-in string
Changes since v2:
    * remove non-abstract socket code path to simplify functions
    * rename lxc_af_unix_* family to lxc_abstract_unix_*

Signed-off-by: S.Çağlar Onur <caglar@10ur.org>
Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>

src/lxc/af_unix.c

@@ -20,6 +20,7 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
+#include <stddef.h>
 #include <string.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -34,7 +35,7 @@
 
 lxc_log_define(lxc_af_unix, lxc);
 
-int lxc_af_unix_open(const char *path, int type, int flags)
+int lxc_abstract_unix_open(const char *path, int type, int flags)
 {
 	int fd;
 	size_t len;
@@ -49,27 +50,26 @@ int lxc_af_unix_open(const char *path, int type, int flags)
 	if (fd < 0)
 		return -1;
 
+	/* Clear address structure */
 	memset(&addr, 0, sizeof(addr));
 
 	if (!path)
 		return fd;
 
 	addr.sun_family = AF_UNIX;
-	/* copy entire buffer in case of abstract socket */
-	len = sizeof(addr.sun_path);
-	if (path[0]) {
-		len = strlen(path);
-		if (len >= sizeof(addr.sun_path)) {
-			process_lock();
-			close(fd);
-			process_unlock();
-			errno = ENAMETOOLONG;
-			return -1;
-		}
+
+	len = strlen(&path[1]) + 1;
+	if (len >= sizeof(addr.sun_path) - 1) {
+		process_lock();
+		close(fd);
+		process_unlock();
+		errno = ENAMETOOLONG;
+		return -1;
 	}
-	memcpy(addr.sun_path, path, len);
+	/* addr.sun_path[0] has already been set to 0 by memset() */
+	strncpy(&addr.sun_path[1], &path[1], strlen(&path[1]));
 
-	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr))) {
+	if (bind(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len)) {
 		int tmp = errno;
 		process_lock();
 		close(fd);
@@ -90,7 +90,7 @@ int lxc_af_unix_open(const char *path, int type, int flags)
 	return fd;
 }
 
-int lxc_af_unix_close(int fd)
+int lxc_abstract_unix_close(int fd)
 {
 	struct sockaddr_un addr;
 	socklen_t addrlen = sizeof(addr);
@@ -106,9 +106,10 @@ int lxc_af_unix_close(int fd)
 	return 0;
 }
 
-int lxc_af_unix_connect(const char *path)
+int lxc_abstract_unix_connect(const char *path)
 {
 	int fd;
+	size_t len;
 	struct sockaddr_un addr;
 
 	process_lock();
@@ -120,11 +121,19 @@ int lxc_af_unix_connect(const char *path)
 	memset(&addr, 0, sizeof(addr));
 
 	addr.sun_family = AF_UNIX;
-	/* copy entire buffer in case of abstract socket */
-	memcpy(addr.sun_path, path,
-	       path[0]?strlen(path):sizeof(addr.sun_path));
 
-	if (connect(fd, (struct sockaddr *)&addr, sizeof(addr))) {
+	len = strlen(&path[1]) + 1;
+	if (len >= sizeof(addr.sun_path) - 1) {
+		process_lock();
+		close(fd);
+		process_unlock();
+		errno = ENAMETOOLONG;
+		return -1;
+	}
+	/* addr.sun_path[0] has already been set to 0 by memset() */
+	strncpy(&addr.sun_path[1], &path[1], strlen(&path[1]));
+
+	if (connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len)) {
 		int tmp = errno;
 		process_lock();
 		close(fd);
@@ -136,7 +145,7 @@ int lxc_af_unix_connect(const char *path)
 	return fd;
 }
 
-int lxc_af_unix_send_fd(int fd, int sendfd, void *data, size_t size)
+int lxc_abstract_unix_send_fd(int fd, int sendfd, void *data, size_t size)
 {
         struct msghdr msg = { 0 };
         struct iovec iov;
@@ -166,7 +175,7 @@ int lxc_af_unix_send_fd(int fd, int sendfd, void *data, size_t size)
         return sendmsg(fd, &msg, 0);
 }
 
-int lxc_af_unix_recv_fd(int fd, int *recvfd, void *data, size_t size)
+int lxc_abstract_unix_recv_fd(int fd, int *recvfd, void *data, size_t size)
 {
         struct msghdr msg = { 0 };
         struct iovec iov;
@@ -205,7 +214,7 @@ int lxc_af_unix_recv_fd(int fd, int *recvfd, void *data, size_t size)
         return ret;
 }
 
-int lxc_af_unix_send_credential(int fd, void *data, size_t size)
+int lxc_abstract_unix_send_credential(int fd, void *data, size_t size)
 {
         struct msghdr msg = { 0 };
         struct iovec iov;
@@ -238,7 +247,7 @@ int lxc_af_unix_send_credential(int fd, void *data, size_t size)
         return sendmsg(fd, &msg, 0);
 }
 
-int lxc_af_unix_rcv_credential(int fd, void *data, size_t size)
+int lxc_abstract_unix_rcv_credential(int fd, void *data, size_t size)
 {
         struct msghdr msg = { 0 };
         struct iovec iov;

src/lxc/af_unix.h

@@ -21,11 +21,11 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-extern int lxc_af_unix_open(const char *path, int type, int flags);
-extern int lxc_af_unix_close(int fd);
-extern int lxc_af_unix_connect(const char *path);
-extern int lxc_af_unix_send_fd(int fd, int sendfd, void *data, size_t size);
-extern int lxc_af_unix_recv_fd(int fd, int *recvfd, void *data, size_t size);
-extern int lxc_af_unix_send_credential(int fd, void *data, size_t size);
-extern int lxc_af_unix_rcv_credential(int fd, void *data, size_t size);
+extern int lxc_abstract_unix_open(const char *path, int type, int flags);
+extern int lxc_abstract_unix_close(int fd);
+extern int lxc_abstract_unix_connect(const char *path);
+extern int lxc_abstract_unix_send_fd(int fd, int sendfd, void *data, size_t size);
+extern int lxc_abstract_unix_recv_fd(int fd, int *recvfd, void *data, size_t size);
+extern int lxc_abstract_unix_send_credential(int fd, void *data, size_t size);
+extern int lxc_abstract_unix_rcv_credential(int fd, void *data, size_t size);
 

src/lxc/commands.c

@@ -136,7 +136,7 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd)
 	int ret,rspfd;
 	struct lxc_cmd_rsp *rsp = &cmd->rsp;
 
-	ret = lxc_af_unix_recv_fd(sock, &rspfd, rsp, sizeof(*rsp));
+	ret = lxc_abstract_unix_recv_fd(sock, &rspfd, rsp, sizeof(*rsp));
 	if (ret < 0) {
 		ERROR("command %s failed to receive response",
 		      lxc_cmd_str(cmd->req.cmd));
@@ -251,7 +251,7 @@ static int lxc_cmd(const char *name, struct lxc_cmd_rr *cmd, int *stopped,
 	if (fill_sock_name(offset, len, name, lxcpath))
 		return -1;
 
-	sock = lxc_af_unix_connect(path);
+	sock = lxc_abstract_unix_connect(path);
 	if (sock < 0) {
 		if (errno == ECONNREFUSED)
 			*stopped = 1;
@@ -261,7 +261,7 @@ static int lxc_cmd(const char *name, struct lxc_cmd_rr *cmd, int *stopped,
 		return -1;
 	}
 
-	ret = lxc_af_unix_send_credential(sock, &cmd->req, sizeof(cmd->req));
+	ret = lxc_abstract_unix_send_credential(sock, &cmd->req, sizeof(cmd->req));
 	if (ret != sizeof(cmd->req)) {
 		SYSERROR("command %s failed to send req to '@%s' %d",
 			 lxc_cmd_str(cmd->req.cmd), offset, ret);
@@ -702,7 +702,7 @@ static int lxc_cmd_console_callback(int fd, struct lxc_cmd_req *req,
 
 	memset(&rsp, 0, sizeof(rsp));
 	rsp.data = INT_TO_PTR(ttynum);
-	if (lxc_af_unix_send_fd(fd, masterfd, &rsp, sizeof(rsp)) < 0) {
+	if (lxc_abstract_unix_send_fd(fd, masterfd, &rsp, sizeof(rsp)) < 0) {
 		ERROR("failed to send tty to client");
 		lxc_console_free(handler->conf, fd);
 		goto out_close;
@@ -758,7 +758,7 @@ static int lxc_cmd_handler(int fd, void *data, struct lxc_epoll_descr *descr)
 	struct lxc_cmd_req req;
 	struct lxc_handler *handler = data;
 
-	ret = lxc_af_unix_rcv_credential(fd, &req, sizeof(req));
+	ret = lxc_abstract_unix_rcv_credential(fd, &req, sizeof(req));
 	if (ret == -EACCES) {
 		/* we don't care for the peer, just send and close */
 		struct lxc_cmd_rsp rsp = { .ret = ret };
@@ -867,7 +867,7 @@ int lxc_cmd_init(const char *name, struct lxc_handler *handler,
 	if (fill_sock_name(offset, len, name, lxcpath))
 		return -1;
 
-	fd = lxc_af_unix_open(path, SOCK_STREAM, 0);
+	fd = lxc_abstract_unix_open(path, SOCK_STREAM, 0);
 	if (fd < 0) {
 		ERROR("failed (%d) to create the command service point %s", errno, offset);
 		if (errno == EADDRINUSE) {

src/lxc/lxc_monitord.c

@@ -210,7 +210,7 @@ static int lxc_monitord_sock_create(struct lxc_monitor *mon)
 	if (lxc_monitor_sock_name(mon->lxcpath, &addr) < 0)
 		return -1;
 
-	fd = lxc_af_unix_open(addr.sun_path, SOCK_STREAM, O_TRUNC);
+	fd = lxc_abstract_unix_open(addr.sun_path, SOCK_STREAM, O_TRUNC);
 	if (fd < 0) {
 		ERROR("failed to open unix socket : %s", strerror(errno));
 		return -1;

src/lxc/monitor.c

@@ -27,6 +27,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <fcntl.h>
 #include <inttypes.h>
 #include <stdint.h>
@@ -194,6 +195,7 @@ int lxc_monitor_open(const char *lxcpath)
 	struct sockaddr_un addr;
 	int fd,ret;
 	int retry,backoff_ms[] = {10, 50, 100};
+	size_t len;
 
 	if (lxc_monitor_sock_name(lxcpath, &addr) < 0)
 		return -1;
@@ -206,8 +208,15 @@ int lxc_monitor_open(const char *lxcpath)
 		return -1;
 	}
 
+	len = strlen(&addr.sun_path[1]) + 1;
+	if (len >= sizeof(addr.sun_path) - 1) {
+		ret = -1;
+		errno = ENAMETOOLONG;
+		goto err1;
+	}
+
 	for (retry = 0; retry < sizeof(backoff_ms)/sizeof(backoff_ms[0]); retry++) {
-		ret = connect(fd, (struct sockaddr *)&addr, sizeof(addr));
+		ret = connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len);
 		if (ret == 0 || errno != ECONNREFUSED)
 			break;
 		ERROR("connect : backing off %d", backoff_ms[retry]);