On 2023/05/25 15:06, Claudio Jeker wrote: > 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.
Thanks for looking into this. OK. > Triggering this bug is not trivial so it is hard test but the problem is > obvious. indeed, I don't think I have hit this one at all before. Running sessions over routes maintained by ospf6d seems a fairly good way to trigger a number of issues ;) > :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); >