summary |
shortlog |
log |
commit | commitdiff |
tree
raw |
patch |
inline | side by side (from parent 1:
be812f2)
The implementation of "ofctl/block" used a nested poll loop, with an inner
call to unixctl_server_run(). This poll loop always ran inside an outer
call to unixctl_server_run(), since that's the context within which unixctl
command implementations run. That means that, if a unixctl connection got
closed within the inner poll loop, and the outer poll loop happened to be
processing the same unixctl connection, then the outer poll loop would
dereference data in the freed connection.
The simplest solution is to avoid a nested poll loop, so that's what this
commit does.
This didn't cause a failure in the unit tests on i386 (which is why I
didn't catch it before pushing) but it did, reliably, on x86-64, and it
showed up in valgrind everywhere.
Signed-off-by: Ben Pfaff <blp@nicira.com>
unixctl_command_reply(conn, NULL);
}
unixctl_command_reply(conn, NULL);
}
-struct block_aux {
- struct vconn *vconn;
- struct unixctl_server *server;
- bool blocked;
-};
-
static void
ofctl_block(struct unixctl_conn *conn, int argc OVS_UNUSED,
static void
ofctl_block(struct unixctl_conn *conn, int argc OVS_UNUSED,
- const char *argv[] OVS_UNUSED, void *block_)
+ const char *argv[] OVS_UNUSED, void *blocked_)
- struct block_aux *block = block_;
+ bool *blocked = blocked_;
+ if (!*blocked) {
+ *blocked = true;
+ unixctl_command_reply(conn, NULL);
+ } else {
unixctl_command_reply(conn, "already blocking");
unixctl_command_reply(conn, "already blocking");
- return;
- }
-
- block->blocked = true;
- unixctl_command_reply(conn, NULL);
- for (;;) {
- unixctl_server_run(block->server);
- if (!block->blocked) {
- break;
- }
- vconn_run(block->vconn);
-
- unixctl_server_wait(block->server);
- vconn_run_wait(block->vconn);
- poll_block();
}
}
static void
ofctl_unblock(struct unixctl_conn *conn, int argc OVS_UNUSED,
}
}
static void
ofctl_unblock(struct unixctl_conn *conn, int argc OVS_UNUSED,
- const char *argv[] OVS_UNUSED, void *block_)
+ const char *argv[] OVS_UNUSED, void *blocked_)
- struct block_aux *block = block_;
+ bool *blocked = blocked_;
- if (!block->blocked) {
- unixctl_command_reply(conn, "not blocking");
- } else {
- block->blocked = false;
+ if (*blocked) {
+ *blocked = false;
unixctl_command_reply(conn, NULL);
unixctl_command_reply(conn, NULL);
+ } else {
+ unixctl_command_reply(conn, "already unblocked");
monitor_vconn(struct vconn *vconn)
{
struct barrier_aux barrier_aux = { vconn, NULL };
monitor_vconn(struct vconn *vconn)
{
struct barrier_aux barrier_aux = { vconn, NULL };
- struct block_aux block;
struct unixctl_server *server;
bool exiting = false;
struct unixctl_server *server;
bool exiting = false;
int error;
daemon_save_fd(STDERR_FILENO);
int error;
daemon_save_fd(STDERR_FILENO);
unixctl_command_register("ofctl/set-output-file", "FILE", 1, 1,
ofctl_set_output_file, NULL);
unixctl_command_register("ofctl/set-output-file", "FILE", 1, 1,
ofctl_set_output_file, NULL);
- block.vconn = vconn;
- block.server = server;
- block.blocked = false;
- unixctl_command_register("ofctl/block", "", 0, 0, ofctl_block, &block);
- unixctl_command_register("ofctl/unblock", "", 0, 0, ofctl_unblock, &block);
+ unixctl_command_register("ofctl/block", "", 0, 0, ofctl_block, &blocked);
+ unixctl_command_register("ofctl/unblock", "", 0, 0, ofctl_unblock,
+ &blocked);
int retval;
unixctl_server_run(server);
int retval;
unixctl_server_run(server);
uint8_t msg_type;
retval = vconn_recv(vconn, &b);
uint8_t msg_type;
retval = vconn_recv(vconn, &b);
vconn_run(vconn);
vconn_run_wait(vconn);
vconn_run(vconn);
vconn_run_wait(vconn);
- vconn_recv_wait(vconn);
+ if (!blocked) {
+ vconn_recv_wait(vconn);
+ }
unixctl_server_wait(server);
poll_block();
}
unixctl_server_wait(server);
poll_block();
}