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

Pavithra Ramesh paramesh at vmware.com
Fri Feb 8 17:51:33 UTC 2013


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