Skip to content

Commit 4cb586a

Browse files
david-marchandigsilya
authored andcommitted
ovs-ofctl: Fix crash with control message errors.
When a bundle transaction fails with multiple errors, bundle_print_errors() crashes with SIGSEGV if an error's XID doesn't match any request in the requests list. This happens because LIST_FOR_EACH_CONTINUE sets bmsg to NULL when it exhausts the list without finding a match. On the next error iteration, the list evaluation crashes with a NULL pointer dereference. Control message errors (TIMEOUT for OPEN_REQUEST, BAD_ID for COMMIT, etc...) have XIDs that never match entries in the requests list. When the bundle times out and a subsequent error is triggered, both errors are collected, triggering the crash. Fix by checking if bmsg is NULL before entering the inner loop. The existing fallback to the truncated payload from the error itself handles the non-matching case correctly. Fixes: d456608 ("list: use multi-variable helpers for list loops.") Reported-at: https://redhat.atlassian.net/browse/FDP-4392 Acked-by: Eelco Chaudron <echaudron@ovn.org> Signed-off-by: David Marchand <david.marchand@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
1 parent 1a613ce commit 4cb586a

1 file changed

Lines changed: 15 additions & 12 deletions

File tree

‎utilities/ovs-ofctl.c‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -763,18 +763,21 @@ bundle_print_errors(struct ovs_list *errors, struct ovs_list *requests,
763763
const struct ofp_header *ofp_msg = payload.data;
764764
size_t msg_len = payload.size;
765765

766-
/* Find the failing message from the requests list to be able to
767-
* dump the whole message. We assume the errors are returned in
768-
* the same order as in which the messages are sent to get O(n)
769-
* rather than O(n^2) processing here. If this heuristics fails we
770-
* may print the truncated hexdumps instead. */
771-
LIST_FOR_EACH_CONTINUE (bmsg, list_node, requests) {
772-
const struct ofp_header *oh = bmsg->data;
773-
774-
if (oh->xid == error_xid) {
775-
ofp_msg = oh;
776-
msg_len = bmsg->size;
777-
break;
766+
if (bmsg) {
767+
/* Find the failing message from the requests list to be able
768+
* to dump the whole message. We assume the errors are
769+
* returned in the same order as in which the messages are sent
770+
* to get O(n) rather than O(n^2) processing here. If this
771+
* heuristics fails we may print the truncated hexdumps
772+
* instead. */
773+
LIST_FOR_EACH_CONTINUE (bmsg, list_node, requests) {
774+
const struct ofp_header *oh = bmsg->data;
775+
776+
if (oh->xid == error_xid) {
777+
ofp_msg = oh;
778+
msg_len = bmsg->size;
779+
break;
780+
}
778781
}
779782
}
780783
fprintf(stderr, "Error %s for: ", ofperr_get_name(ofperr));

0 commit comments

Comments
 (0)