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

[Serge Hallyn]

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
> [...]

Yeah I've noticed that too :)  However, you can't just take the length
of the passed-in string, you need to make sure it's no larger
than sizeof(addr.sun_path)-1.  Is that being guaranteed somewhere else
that I'm glossing over?

> Signed-off-by: S.Çağlar Onur <caglar at 10ur.org>
> ---
>  src/lxc/af_unix.c | 23 +++++++++++++++--------
>  src/lxc/monitor.c |  5 ++++-
>  2 files changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c
> index 333f05e..2f8d593 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>
> @@ -55,8 +56,6 @@ int lxc_af_unix_open(const char *path, int type, int flags)
>  		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)) {
> @@ -66,10 +65,13 @@ int lxc_af_unix_open(const char *path, int type, int flags)
>  			errno = ENAMETOOLONG;
>  			return -1;
>  		}
> +		memcpy(addr.sun_path, path, len);
> +	} else {
> +		len = offsetof(struct sockaddr_un, sun_path) + strlen(&path[1]) + 1;
> +		memcpy((char *) &addr.sun_path + 1, &path[1], len);
>  	}
> -	memcpy(addr.sun_path, path, len);
>  
> -	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr))) {
> +	if (bind(fd, (struct sockaddr *)&addr, len)) {
>  		int tmp = errno;
>  		process_lock();
>  		close(fd);
> @@ -109,6 +111,7 @@ int lxc_af_unix_close(int fd)
>  int lxc_af_unix_connect(const char *path)
>  {
>  	int fd;
> +	size_t len;
>  	struct sockaddr_un addr;
>  
>  	process_lock();
> @@ -120,11 +123,15 @@ 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 (path[0]) {
> +		len = strlen(path);
> +		memcpy(addr.sun_path, path, len);
> +	} else {
> +		len = offsetof(struct sockaddr_un, sun_path) + strlen(&path[1]) + 1;
> +		memcpy((char *) &addr.sun_path + 1, &path[1], len); 
> +	}
>  
> -	if (connect(fd, (struct sockaddr *)&addr, sizeof(addr))) {
> +	if (connect(fd, (struct sockaddr *)&addr, len)) {
>  		int tmp = errno;
>  		process_lock();
>  		close(fd);
> diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
> index e736937..b965225 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,9 @@ int lxc_monitor_open(const char *lxcpath)
>  		return -1;
>  	}
>  
> +	len = offsetof(struct sockaddr_un, sun_path) + strlen(&addr.sun_path[1]) + 1;
>  	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, 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