[lxc-devel] [PATCH] Use actual length of socket's name for abstract sockets (v3)

S.Çağlar Onur caglar at 10ur.org
Fri Oct 25 22:04:52 UTC 2013


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 at 10ur.org>
---
 src/lxc/af_unix.c      | 57 +++++++++++++++++++++++++++++---------------------
 src/lxc/af_unix.h      | 14 ++++++-------
 src/lxc/commands.c     | 12 +++++------
 src/lxc/lxc_monitord.c |  2 +-
 src/lxc/monitor.c      | 11 +++++++++-
 5 files changed, 57 insertions(+), 39 deletions(-)

diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c
index 333f05e..ab73963 100644
--- a/src/lxc/af_unix.c
+++ b/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 @@ out:
         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;
diff --git a/src/lxc/af_unix.h b/src/lxc/af_unix.h
index 6bc253d..81f2986 100644
--- a/src/lxc/af_unix.h
+++ b/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);
 
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 3e44ef3..63adaf5 100644
--- a/src/lxc/commands.c
+++ b/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) {
diff --git a/src/lxc/lxc_monitord.c b/src/lxc/lxc_monitord.c
index fdc80c5..11936a9 100644
--- a/src/lxc/lxc_monitord.c
+++ b/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;
diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index ab567c8..1eae8e6 100644
--- a/src/lxc/monitor.c
+++ b/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]);
-- 
1.8.3.2





More information about the lxc-devel mailing list