[lxc-devel] [PATCH] Use actual length of socket's name for abstract sockets
S.Çağlar Onur
caglar at 10ur.org
Tue Oct 22 02:18:10 UTC 2013
Hi,
On Mon, Oct 21, 2013 at 8:30 PM, Serge Hallyn <serge.hallyn at ubuntu.com>wrote:
> 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?
Hmm I think current code path also lacks that check. As long as I see we
only control the length in lxc_af_unix_open for non-abstract case. I'll add
the checks and iterate one more time sometime this week.
> 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
>
--
S.Çağlar Onur <caglar at 10ur.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20131021/7137cdea/attachment.html>
More information about the lxc-devel
mailing list