Hi,
On Mon, Oct 21, 2013 at 8:30 PM, Serge Hallyn <serge.hal...@ubuntu.com>wrote:
> 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
> > [...]
>
> 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 <cag...@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@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/lxc-devel
>
--
S.Çağlar Onur <cag...@10ur.org>
------------------------------------------------------------------------------
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
