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>

diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c
index 333f05eb34b293614ebbc5192df42ed709b6b407..ab73963e3a170a3aa4faf6695168666bc4fb17b8 100644 (file)
--- 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 6bc253dd01d1fbbdb39db18008358fc8f68113a8..81f2986968f935a86d65e6d9aaccf6ef9f2d4198 100644 (file)
--- 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 3e44ef3b8e932b9179b11da57bfb37b698b078df..63adaf56cbb1f601098c345735feee48ff732850 100644 (file)
--- 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 fdc80c5fdacdbaae24c459dae767a3428537072e..11936a915ca802c81804ed751fad8c2967962f83 100644 (file)
--- 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 ab567c81b0fe372a285a850e13ec22ada6ebc0ae..1eae8e625db9049b5211005f9d5ca0d785d5f72f 100644 (file)
--- 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]);