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

Serge Hallyn serge.hallyn at ubuntu.com
Fri Oct 25 21:05:14 UTC 2013


Quoting S.Çağlar Onur (caglar at 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 <caglar at 10ur.org>

Acked-by: Serge E. Hallyn <serge.hallyn at 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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel




More information about the lxc-devel mailing list