Investigation found that Some of the occasional failures in the
"ovn -- vtep: 3 HVs, 1 VIFs/HV, 1 GW, 1 LS" test case are caused
by ovs-vswitchd crashing with SIGSEGV. It turns out that the
crash occurrs when the number of netdev-dummy passive connections
transitions from 1 to 2. When xrealloc() copies the array of
dummy_packet_stream structures from the original buffer to a
newly allocated one, the struct ovs_list txq member of the structure
becomes corrupt (e.g. if ovs_list_is_empty() would have returned
false before the copy, it will return true after the copy, which
will lead to a crash when the bogus packet buffer on the list is
dereferenced).
Fix by taking a hint from David Wheeler and adding a level of
indirection.
Signed-off-by: Lance Richardson <lrichard@redhat.com>
[blp@ovn.org folded in an additional bug fix]
Signed-off-by: Ben Pfaff <blp@ovn.org>
struct dummy_packet_pconn {
struct pstream *pstream;
struct dummy_packet_pconn {
struct pstream *pstream;
- struct dummy_packet_stream *streams;
+ struct dummy_packet_stream **streams;
case PASSIVE:
pstream_close(pconn->pstream);
for (i = 0; i < pconn->n_streams; i++) {
case PASSIVE:
pstream_close(pconn->pstream);
for (i = 0; i < pconn->n_streams; i++) {
- dummy_packet_stream_close(&pconn->streams[i]);
+ dummy_packet_stream_close(pconn->streams[i]);
+ free(pconn->streams[i]);
}
free(pconn->streams);
pconn->pstream = NULL;
}
free(pconn->streams);
pconn->pstream = NULL;
pconn->streams = xrealloc(pconn->streams,
((pconn->n_streams + 1)
pconn->streams = xrealloc(pconn->streams,
((pconn->n_streams + 1)
- * sizeof *s));
- s = &pconn->streams[pconn->n_streams++];
+ * sizeof s));
+ s = xmalloc(sizeof *s);
+ pconn->streams[pconn->n_streams++] = s;
dummy_packet_stream_init(s, new_stream);
} else if (error != EAGAIN) {
VLOG_WARN("%s: accept failed (%s)",
dummy_packet_stream_init(s, new_stream);
} else if (error != EAGAIN) {
VLOG_WARN("%s: accept failed (%s)",
- for (i = 0; i < pconn->n_streams; i++) {
- struct dummy_packet_stream *s = &pconn->streams[i];
+ for (i = 0; i < pconn->n_streams; ) {
+ struct dummy_packet_stream *s = pconn->streams[i];
error = dummy_packet_stream_run(dev, s);
if (error) {
error = dummy_packet_stream_run(dev, s);
if (error) {
stream_get_name(s->stream),
ovs_retval_to_string(error));
dummy_packet_stream_close(s);
stream_get_name(s->stream),
ovs_retval_to_string(error));
dummy_packet_stream_close(s);
pconn->streams[i] = pconn->streams[--pconn->n_streams];
pconn->streams[i] = pconn->streams[--pconn->n_streams];
case PASSIVE:
pstream_wait(conn->u.pconn.pstream);
for (i = 0; i < conn->u.pconn.n_streams; i++) {
case PASSIVE:
pstream_wait(conn->u.pconn.pstream);
for (i = 0; i < conn->u.pconn.n_streams; i++) {
- struct dummy_packet_stream *s = &conn->u.pconn.streams[i];
+ struct dummy_packet_stream *s = conn->u.pconn.streams[i];
dummy_packet_stream_wait(s);
}
break;
dummy_packet_stream_wait(s);
}
break;
switch (conn->type) {
case PASSIVE:
for (i = 0; i < conn->u.pconn.n_streams; i++) {
switch (conn->type) {
case PASSIVE:
for (i = 0; i < conn->u.pconn.n_streams; i++) {
- struct dummy_packet_stream *s = &conn->u.pconn.streams[i];
+ struct dummy_packet_stream *s = conn->u.pconn.streams[i];
dummy_packet_stream_send(s, buffer, size);
pstream_wait(conn->u.pconn.pstream);
dummy_packet_stream_send(s, buffer, size);
pstream_wait(conn->u.pconn.pstream);