sthen@ reported a bgpd SE crash to me and after inspection of the report
it looks like he managed to trigger a mistake in session_process_msg().
When for example a NOTIFICATION message is received then the state change
clears the rbuf. Now normally the for loop starts over afterwards and the
if (p->rbuf == NULL) at the top causes the function to return.
In his case the peer had enough messages queued that the if (++processed >
MSG_PROCESS_LIMIT) limit triggered after the NOTIFICATION was processed.
This hits a break from the for loop and the code at the end of the
function adjusting the rbuf trips over the NULL rbuf pointer.
This can be fixed in many ways, I decided to just check at the end of the
loop that rbuf is still valid.
Triggering this bug is not trivial so it is hard test but the problem is
obvious.
--
:wq Claudio
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.444
diff -u -p -r1.444 session.c
--- session.c 5 May 2023 07:28:08 -0000 1.444
+++ session.c 25 May 2023 09:32:11 -0000
@@ -1998,6 +1998,8 @@ session_process_msg(struct peer *p)
}
}
+ if (p->rbuf == NULL)
+ return;
if (rpos < av) {
left = av - rpos;
memmove(&p->rbuf->buf, p->rbuf->buf + rpos, left);
