Skip to content

Commit

Permalink
mctpd: Fix resource handling over recovery of exchanged peer devices
Browse files Browse the repository at this point in the history
A fundamental flaw merged in 7ec2f8d ("mctpd: Add support for
endpoint recovery") was that NULL was passed for the peer parameter in
the call to endpoint_assign_eid(). I had intended to circle back to
that, but it clearly got overlooked in testing and review.

While this caused an obvious crash when the path was hit, the subsequent
resource cleanup was also in error: As the original peer has been
removed there's no need for the remaining state maintenance executed in
the original flow. Instead, immediately return with the result of
endpoint_assign_eid() against the new peer device.

Fixes: 7ec2f8d ("mctpd: Add support for endpoint recovery")
Signed-off-by: Andrew Jeffery <[email protected]>
  • Loading branch information
amboar committed Jan 12, 2024
1 parent 858d203 commit 3a75f27
Showing 1 changed file with 19 additions and 17 deletions.
36 changes: 19 additions & 17 deletions src/mctpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -2303,29 +2303,31 @@ peer_endpoint_recover(sd_event_source *s, uint64_t usec, void *userdata)
uuid_matches_peer = memcmp(uuid, peer->uuid, sizeof(uuid)) == 0;
uuid_matches_nil = memcmp(uuid, nil_uuid, sizeof(uuid)) == 0;

if (rc == 0 && uuid_matches_peer &&
(!uuid_matches_nil || MCTPD_RECOVER_NIL_UUID)) {
/* Confirmation of the same device, apply it's already allocated EID */
rc = endpoint_send_set_endpoint_id(peer, &new_eid);
if (rc < 0) {
goto reschedule;
}

if (new_eid != peer->eid) {
rc = change_peer_eid(peer, new_eid);
if (rc < 0) {
goto reclaim;
}
}
} else {
if (rc || !uuid_matches_peer ||
(uuid_matches_nil && !MCTPD_RECOVER_NIL_UUID)) {
/* It's not known to be the same device, allocate a new EID */
dest_phys phys = peer->phys;

assert(sd_event_source_get_enabled(peer->recovery.source, &ev_state) == 0);
remove_peer(peer);
rc = endpoint_assign_eid(ctx, NULL, &phys, NULL);
/*
* The representation of the old peer is now gone. Set up the new peer,
* after which we immediately return as there's no old peer state left to
* maintain.
*/
return endpoint_assign_eid(ctx, NULL, &phys, &peer);
}

/* Confirmation of the same device, apply its already allocated EID */
rc = endpoint_send_set_endpoint_id(peer, &new_eid);
if (rc < 0) {
goto reschedule;
}

if (new_eid != peer->eid) {
rc = change_peer_eid(peer, new_eid);
if (rc < 0) {
goto reschedule;
goto reclaim;
}
}
}
Expand Down

0 comments on commit 3a75f27

Please sign in to comment.