Skip to content

Commit

Permalink
ospf6d: Stop crash in ospf6_write
Browse files Browse the repository at this point in the history
I'm seeing crashes in ospf6_write on the `assert(node)`.  The only
sequence of events that I see that could possibly cause this to happen
is this:

a) Someone has scheduled a outgoing write to the ospf6->t_write and
placed item(s) on the ospf6->oi_write_q
b) A decision is made in ospf6_send_lsupdate() to send an immediate
packet via a event_execute(..., ospf6_write,....).
c) ospf6_write is called and the oi_write_q is cleaned out.
d) the t_write event is now popped and the oi_write_q is empty
and FRR asserts on the `assert(node)` <crash>

When event_execute is called for ospf6_write, just cancel the t_write
event.  If ospf6_write has more data to send at the end of the function
it will reschedule itself.  I've only seen this crash one time and am
unable to reliably reproduce this at all.  But this is the only mechanism
that I can see that could make this happen, given how little the oi_write_q
is actually touched in code.

Signed-off-by: Donald Sharp <[email protected]>
(cherry picked from commit 3fa39a3)
  • Loading branch information
donaldsharp authored and mergify[bot] committed Jul 2, 2023
1 parent c9a420b commit c509367
Showing 1 changed file with 20 additions and 5 deletions.
25 changes: 20 additions & 5 deletions ospf6d/ospf6_message.c
Original file line number Diff line number Diff line change
Expand Up @@ -2565,9 +2565,7 @@ static void ospf6_send_lsupdate(struct ospf6_neighbor *on,
struct ospf6_interface *oi,
struct ospf6_packet *op)
{

if (on) {

if ((on->ospf6_if->state == OSPF6_INTERFACE_POINTTOPOINT)
|| (on->ospf6_if->state == OSPF6_INTERFACE_DR)
|| (on->ospf6_if->state == OSPF6_INTERFACE_BDR))
Expand All @@ -2584,19 +2582,36 @@ static void ospf6_send_lsupdate(struct ospf6_neighbor *on,
op->dst = alldrouters6;
}
if (oi) {
struct ospf6 *ospf6;

ospf6_fill_hdr_checksum(oi, op);
ospf6_packet_add(oi, op);
/* If ospf instance is being deleted, send the packet
* immediately
*/
if ((oi->area == NULL) || (oi->area->ospf6 == NULL))
return;
if (oi->area->ospf6->inst_shutdown) {

ospf6 = oi->area->ospf6;
if (ospf6->inst_shutdown) {
if (oi->on_write_q == 0) {
listnode_add(oi->area->ospf6->oi_write_q, oi);
listnode_add(ospf6->oi_write_q, oi);
oi->on_write_q = 1;
}
event_execute(master, ospf6_write, oi->area->ospf6, 0);
/*
* When ospf6d immediately calls event_execute
* for items in the oi_write_q. The event_execute
* will call ospf6_write and cause the oi_write_q
* to be emptied. *IF* there is already an event
* scheduled for the oi_write_q by something else
* then when it wakes up in the future and attempts
* to cycle through items in the queue it will
* assert. Let's stop the t_write event and
* if ospf6_write doesn't finish up the work
* it will schedule itself again.
*/
event_cancel(&ospf6->t_write);
event_execute(master, ospf6_write, ospf6, 0);
} else
OSPF6_MESSAGE_WRITE_ON(oi);
}
Expand Down

0 comments on commit c509367

Please sign in to comment.