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" <b...@nicira.com>
> To: "Pavithra Ramesh" <param...@vmware.com>
> Cc: dev@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" <b...@nicira.com>
> > To: "Pavithra Ramesh" <param...@vmware.com>
> > Cc: dev@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 <param...@vmware.com>
> >
> > Here's a revised version that passes all tests.
> >
> > --8<--------------------------cut here-------------------------->8--
> >
> > From: Pavithra Ramesh <param...@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 <param...@vmware.com>
> > Signed-off-by: Ben Pfaff <b...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
