Quoting S.Çağlar Onur (cag...@10ur.org):
> 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 <cag...@10ur.org>

Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>

Note that the added length check in lxc_monitor_open() is not necessary
as it is already enforced at lxc_monitor_sock_name().

> ---
>  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
> 
> 
> ------------------------------------------------------------------------------
> October Webinars: Code for Performance
> Free Intel webinars can help you accelerate application performance.
> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
> the latest Intel processors and coprocessors. See abstracts and register >
> http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel

------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from 
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60135991&iu=/4140/ostg.clktrk
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to