[ovs-dev] [PATCH] stream-unix: append ovs_rundir to socket

Ben Pfaff blp at nicira.com
Fri Feb 8 20:18:35 UTC 2013


Usually we put both the C and Python versions into one patch, if we
remember to change both versions at the same time.

On Fri, Feb 08, 2013 at 11:24:45AM -0800, Pavithra Ramesh wrote:
> Sure, I've made the change and will send out a new patch.
> Can you push this one?
> 
> Thanks,
> Pavithra
> 
> ----- Original Message -----
> From: "Ben Pfaff" <blp at nicira.com>
> To: "Pavithra Ramesh" <paramesh at vmware.com>
> Cc: dev at openvswitch.org
> Sent: Friday, February 8, 2013 11:18:38 AM
> Subject: Re: [ovs-dev] [PATCH] stream-unix: append ovs_rundir to socket
> 
> Yes, would you mind taking a first shot?
> 
> On Fri, Feb 08, 2013 at 09:51:33AM -0800, Pavithra Ramesh wrote:
> > Thanks Ben. The patch looks good.
> > Do we also want to change the python stream implementation
> > to behave the same way? 
> > 
> > -Pavithra
> > 
> > ----- Original Message -----
> > From: "Ben Pfaff" <blp at nicira.com>
> > To: "Pavithra Ramesh" <paramesh at vmware.com>
> > Cc: dev at openvswitch.org
> > Sent: Friday, February 8, 2013 9:14:13 AM
> > Subject: Re: [ovs-dev] [PATCH] stream-unix: append ovs_rundir to socket
> > 
> > On Thu, Feb 07, 2013 at 03:48:14PM -0800, Pavithra Ramesh wrote:
> > > If socket path specified is relative to ovs_rundir(),
> > > append the directory name to in unix_open and punix_open.
> > > Freed the new newly allocated strings.
> > > Also included the change in bridge.c to relax the whitelist
> > > check, only if there is no /.
> > > 
> > > Signed-off-by: Pavithra Ramesh <paramesh at vmware.com>
> > 
> > Here's a revised version that passes all tests.
> > 
> > --8<--------------------------cut here-------------------------->8--
> > 
> > From: Pavithra Ramesh <paramesh at vmware.com>
> > Date: Thu, 7 Feb 2013 15:48:14 -0800
> > Subject: [PATCH] stream-unix: append ovs_rundir to socket
> > 
> > If socket path specified is relative to ovs_rundir(),
> > append the directory name to in unix_open and punix_open.
> > Freed the new newly allocated strings.
> > Also included the change in bridge.c to relax the whitelist
> > check, only if there is no /.
> > 
> > Signed-off-by: Pavithra Ramesh <paramesh at vmware.com>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  lib/stream-unix.c        |   20 ++++++++++++++------
> >  tests/jsonrpc.at         |    3 +++
> >  tests/ovsdb-execution.at |    1 +
> >  tests/ovsdb-idl.at       |    3 +++
> >  tests/ovsdb-macros.at    |    3 ++-
> >  tests/ovsdb-monitor.at   |    1 +
> >  tests/ovsdb-server.at    |    9 +++++++++
> >  tests/ovsdb-tool.at      |    4 ++++
> >  tests/vconn.at           |    1 +
> >  vswitchd/bridge.c        |    6 ++++--
> >  10 files changed, 42 insertions(+), 9 deletions(-)
> > 
> > diff --git a/lib/stream-unix.c b/lib/stream-unix.c
> > index 6ed7648..dbee135 100644
> > --- a/lib/stream-unix.c
> > +++ b/lib/stream-unix.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -29,6 +29,7 @@
> >  #include "packets.h"
> >  #include "poll-loop.h"
> >  #include "socket-util.h"
> > +#include "dirs.h"
> >  #include "util.h"
> >  #include "stream-provider.h"
> >  #include "stream-fd.h"
> > @@ -42,15 +43,19 @@ static int
> >  unix_open(const char *name, char *suffix, struct stream **streamp,
> >            uint8_t dscp OVS_UNUSED)
> >  {
> > -    const char *connect_path = suffix;
> > +    char *connect_path;
> >      int fd;
> >  
> > +    connect_path = abs_file_name(ovs_rundir(), suffix);
> >      fd = make_unix_socket(SOCK_STREAM, true, NULL, connect_path);
> > +
> >      if (fd < 0) {
> >          VLOG_DBG("%s: connection failed (%s)", connect_path, strerror(-fd));
> > +        free(connect_path);
> >          return -fd;
> >      }
> >  
> > +    free(connect_path);
> >      return new_fd_stream(name, fd, check_connection_completion(fd), streamp);
> >  }
> >  
> > @@ -76,11 +81,14 @@ static int
> >  punix_open(const char *name OVS_UNUSED, char *suffix,
> >             struct pstream **pstreamp, uint8_t dscp OVS_UNUSED)
> >  {
> > +    char *bind_path;
> >      int fd, error;
> >  
> > -    fd = make_unix_socket(SOCK_STREAM, true, suffix, NULL);
> > +    bind_path = abs_file_name(ovs_rundir(), suffix);
> > +    fd = make_unix_socket(SOCK_STREAM, true, bind_path, NULL);
> >      if (fd < 0) {
> > -        VLOG_ERR("%s: binding failed: %s", suffix, strerror(errno));
> > +        VLOG_ERR("%s: binding failed: %s", bind_path, strerror(errno));
> > +        free(bind_path);
> >          return errno;
> >      }
> >  
> > @@ -88,11 +96,11 @@ punix_open(const char *name OVS_UNUSED, char *suffix,
> >          error = errno;
> >          VLOG_ERR("%s: listen: %s", name, strerror(error));
> >          close(fd);
> > +        free(bind_path);
> >          return error;
> >      }
> >  
> > -    return new_fd_pstream(name, fd, punix_accept, NULL,
> > -                          xstrdup(suffix), pstreamp);
> > +    return new_fd_pstream(name, fd, punix_accept, NULL, bind_path, pstreamp);
> >  }
> >  
> >  static int
> > diff --git a/tests/jsonrpc.at b/tests/jsonrpc.at
> > index 2a7f91b..664debe 100644
> > --- a/tests/jsonrpc.at
> > +++ b/tests/jsonrpc.at
> > @@ -1,6 +1,7 @@
> >  AT_BANNER([JSON-RPC - C])
> >  
> >  AT_SETUP([JSON-RPC request and successful reply])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  AT_CHECK([test-jsonrpc --detach --no-chdir --pidfile="`pwd`"/pid listen punix:socket])
> >  AT_CHECK([test -s pid])
> >  AT_CHECK([kill -0 `cat pid`])
> > @@ -12,6 +13,7 @@ AT_CHECK([kill `cat pid`])
> >  AT_CLEANUP
> >  
> >  AT_SETUP([JSON-RPC request and error reply])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  AT_CHECK([test-jsonrpc --detach --no-chdir --pidfile="`pwd`"/pid listen punix:socket])
> >  AT_CHECK([test -s pid])
> >  AT_CHECK([kill -0 `cat pid`])
> > @@ -23,6 +25,7 @@ AT_CHECK([kill `cat pid`])
> >  AT_CLEANUP
> >  
> >  AT_SETUP([JSON-RPC notification])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  AT_CHECK([test-jsonrpc --detach --no-chdir --pidfile="`pwd`"/pid listen punix:socket])
> >  AT_CHECK([test -s pid])
> >  # When a daemon dies it deletes its pidfile, so make a copy.
> > diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at
> > index 6a3b5d1..eec2a04 100644
> > --- a/tests/ovsdb-execution.at
> > +++ b/tests/ovsdb-execution.at
> > @@ -138,6 +138,7 @@ m4_divert_pop([PREPARE_TESTS])
> >  m4_define([OVSDB_CHECK_EXECUTION],
> >    [AT_SETUP([$1])
> >     AT_KEYWORDS([ovsdb execute execution positive $5])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     AT_CHECK([test-ovsdb execute "`$2`" m4_foreach([txn], [$3], [ 'txn'])],
> >       [0], [stdout], [])
> >     AT_CHECK([perl $srcdir/uuidfilt.pl stdout], [0], [$4])
> > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> > index ce22010..a206f62 100644
> > --- a/tests/ovsdb-idl.at
> > +++ b/tests/ovsdb-idl.at
> > @@ -19,6 +19,7 @@ AT_BANNER([OVSDB -- interface description language (IDL)])
> >  m4_define([OVSDB_CHECK_IDL_C],
> >    [AT_SETUP([$1 - C])
> >     AT_KEYWORDS([ovsdb server idl positive $5])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     AT_CHECK([ovsdb-tool create db $abs_srcdir/idltest.ovsschema],
> >                    [0], [stdout], [ignore])
> >     AT_CHECK([ovsdb-server '-vPATTERN:console:ovsdb-server|%c|%m' --detach --no-chdir --pidfile="`pwd`"/pid --remote=punix:socket --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
> > @@ -36,6 +37,7 @@ m4_define([OVSDB_CHECK_IDL_PY],
> >    [AT_SETUP([$1 - Python])
> >     AT_SKIP_IF([test $HAVE_PYTHON = no])
> >     AT_KEYWORDS([ovsdb server idl positive Python $5])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     AT_CHECK([ovsdb-tool create db $abs_srcdir/idltest.ovsschema],
> >                    [0], [stdout], [ignore])
> >     AT_CHECK([ovsdb-server '-vPATTERN:console:ovsdb-server|%c|%m' --detach --no-chdir --pidfile="`pwd`"/pid --remote=punix:socket --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
> > @@ -53,6 +55,7 @@ m4_define([OVSDB_CHECK_IDL_TCP_PY],
> >    [AT_SETUP([$1 - Python tcp])
> >     AT_SKIP_IF([test $HAVE_PYTHON = no])
> >     AT_KEYWORDS([ovsdb server idl positive Python with tcp socket $5])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     AT_CHECK([ovsdb-tool create db $abs_srcdir/idltest.ovsschema],
> >                    [0], [stdout], [ignore])
> >     AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
> > diff --git a/tests/ovsdb-macros.at b/tests/ovsdb-macros.at
> > index c1aa619..2aa752b 100644
> > --- a/tests/ovsdb-macros.at
> > +++ b/tests/ovsdb-macros.at
> > @@ -2,7 +2,8 @@ dnl OVSDB_INIT([$1])
> >  dnl
> >  dnl Creates an empty database named $1.
> >  m4_define([OVSDB_INIT],
> > -  [AT_CHECK(
> > +  [OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> > +   AT_CHECK(
> >       [ovsdb-tool create $1 $abs_top_srcdir/vswitchd/vswitch.ovsschema],
> >       [0], [stdout], [ignore])
> >     AT_CHECK(
> > diff --git a/tests/ovsdb-monitor.at b/tests/ovsdb-monitor.at
> > index 167b44c..aff5854 100644
> > --- a/tests/ovsdb-monitor.at
> > +++ b/tests/ovsdb-monitor.at
> > @@ -19,6 +19,7 @@ AT_BANNER([OVSDB -- ovsdb-server monitors])
> >  m4_define([OVSDB_CHECK_MONITOR], 
> >    [AT_SETUP([$1])
> >     AT_KEYWORDS([ovsdb server monitor positive $9])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     $2 > schema
> >     AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
> >     m4_foreach([txn], [$3],
> > diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
> > index 6dcf2f5..c5feec9 100644
> > --- a/tests/ovsdb-server.at
> > +++ b/tests/ovsdb-server.at
> > @@ -22,6 +22,7 @@ m4_define([OVSDB_SERVER_SHUTDOWN],
> >  m4_define([OVSDB_CHECK_EXECUTION], 
> >    [AT_SETUP([$1])
> >     AT_KEYWORDS([ovsdb server positive unix $5])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     $2 > schema
> >     AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
> >     AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid --remote=punix:socket --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore])
> > @@ -39,6 +40,7 @@ EXECUTION_EXAMPLES
> >  
> >  AT_SETUP([truncating corrupted database log])
> >  AT_KEYWORDS([ovsdb server positive unix])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  ordinal_schema > schema
> >  AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
> >  dnl Do one transaction and save the output.
> > @@ -85,6 +87,7 @@ AT_CLEANUP
> >  
> >  AT_SETUP([truncating database log with bad transaction])
> >  AT_KEYWORDS([ovsdb server positive unix])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  ordinal_schema > schema
> >  AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
> >  dnl Do one transaction and save the output.
> > @@ -132,6 +135,7 @@ AT_CLEANUP
> >  
> >  AT_SETUP([ovsdb-client get-schema-version])
> >  AT_KEYWORDS([ovsdb server positive])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  ordinal_schema > schema
> >  AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore])
> >  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid --unixctl="`pwd`"/unixctl --remote=punix:socket db], [0], [ignore], [ignore])
> > @@ -142,6 +146,7 @@ AT_CLEANUP
> >  
> >  AT_SETUP([database multiplexing implementation])
> >  AT_KEYWORDS([ovsdb server positive])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  ordinal_schema > schema1
> >  constraint_schema > schema2
> >  AT_CHECK([ovsdb-tool create db1 schema1], [0], [ignore], [ignore])
> > @@ -280,6 +285,7 @@ AT_CLEANUP
> >  
> >  AT_SETUP([compacting online])
> >  AT_KEYWORDS([ovsdb server compact])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  ordinal_schema > schema
> >  dnl Make sure that "ovsdb-tool create" works with a dangling symlink for
> >  dnl the database and the lockfile, creating the target of each symlink rather
> > @@ -430,6 +436,7 @@ m4_define([OVSDB_CHECK_EXECUTION],
> >    [AT_SETUP([$1])
> >     AT_KEYWORDS([ovsdb server positive ssl $5])
> >     AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     $2 > schema
> >     AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
> >     SSL_PORT=`cat stdout`
> > @@ -479,6 +486,7 @@ AT_CLEANUP])
> >  m4_define([OVSDB_CHECK_EXECUTION],
> >    [AT_SETUP([$1])
> >     AT_KEYWORDS([ovsdb server positive tcp $5])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     $2 > schema
> >     AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout])
> >     TCP_PORT=`cat stdout`
> > @@ -519,6 +527,7 @@ AT_BANNER([OVSDB -- transactions on transient ovsdb-server])
> >  m4_define([OVSDB_CHECK_EXECUTION], 
> >    [AT_SETUP([$1])
> >     AT_KEYWORDS([ovsdb server positive transient $5])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     $2 > schema
> >     AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
> >     m4_foreach([txn], [$3], 
> > diff --git a/tests/ovsdb-tool.at b/tests/ovsdb-tool.at
> > index e4f4a29..286d700 100644
> > --- a/tests/ovsdb-tool.at
> > +++ b/tests/ovsdb-tool.at
> > @@ -16,6 +16,7 @@ AT_BANNER([OVSDB -- ovsdb-tool])
> >  m4_define([OVSDB_CHECK_EXECUTION], 
> >    [AT_SETUP([$1])
> >     AT_KEYWORDS([ovsdb file positive $5])
> > +   OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >     $2 > schema
> >     touch .db.~lock~
> >     AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
> > @@ -48,6 +49,7 @@ AT_CLEANUP
> >  
> >  AT_SETUP([ovsdb-tool compact])
> >  AT_KEYWORDS([ovsdb file positive])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  ordinal_schema > schema
> >  dnl Make sure that "ovsdb-tool create" works with a dangling symlink,
> >  dnl creating the target of the symlink rather than replacing the symlink
> > @@ -155,6 +157,7 @@ AT_CLEANUP
> >  
> >  AT_SETUP([ovsdb-tool convert -- removing a column])
> >  AT_KEYWORDS([ovsdb file positive])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  ordinal_schema > schema
> >  AT_DATA([new-schema], 
> >    [[{"name": "ordinals",
> > @@ -218,6 +221,7 @@ AT_CLEANUP
> >  
> >  AT_SETUP([ovsdb-tool convert -- adding a column])
> >  AT_KEYWORDS([ovsdb file positive])
> > +OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >  AT_DATA([schema], 
> >    [[{"name": "ordinals",
> >       "tables": {
> > diff --git a/tests/vconn.at b/tests/vconn.at
> > index ae095b0..67a4224 100644
> > --- a/tests/vconn.at
> > +++ b/tests/vconn.at
> > @@ -11,6 +11,7 @@ m4_define([TEST_VCONN_CLASS],
> >        [send-short-hello],
> >        [send-invalid-version-hello]],
> >       [AT_SETUP([$1 vconn - m4_bpatsubst(testname, [-], [ ])])
> > +      OVS_RUNDIR=`pwd`; export OVS_RUNDIR
> >        m4_if([$1], [ssl], [
> >          AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
> >          AT_CHECK([cp $abs_top_builddir/tests/testpki*.pem .])])
> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> > index f5a4366..fdd7c64 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -2799,8 +2799,10 @@ bridge_configure_remotes(struct bridge *br,
> >              if (!strncmp(c->target, "unix:", 5)) {
> >                  /* Connect to a listening socket */
> >                  whitelist = xasprintf("unix:%s/", ovs_rundir());
> > -                if (!equal_pathnames(c->target, whitelist,
> > -                                     strlen(whitelist))) {
> > +                if (strchr(c->target, '/') &&
> > +                   !equal_pathnames(c->target, whitelist,
> > +                     strlen(whitelist))) {
> > +                    /* Absolute path specified, but not in ovs_rundir */
> >                      VLOG_ERR_RL(&rl, "bridge %s: Not connecting to socket "
> >                                    "controller \"%s\" due to possibility for "
> >                                    "remote exploit.  Instead, specify socket "
> > -- 
> > 1.7.10.4



More information about the dev mailing list