[cmaster-next] [PATCH] bgpd: fix invalid memory access in peer_free()

[Renato Westphal]

Agreed, I think cmaster-next-renato is ready to be merged.

Martin did a full ANVL run on this branch and the results look good now:
https://drive.google.com/drive/folders/0B8W_T0dxQfwxdXo4STRKTG9SR0U?usp=sharing



On Thu, Dec 1, 2016 at 1:47 PM, Donald Sharp <sharpd at cumulusnetworks.com> wrote:
> I think memory leaks fixing is good for a release candidate.
>
> donald
>
> On Thu, Dec 1, 2016 at 10:38 AM, David Lamparter
> <david at opensourcerouting.org> wrote:
>> I've applied it on stable/2.0 even though we don't have the "plug
>> memleaks" patch there.  (Maybe we should?)
>>
>> -David
>>
>> On Mon, Nov 28, 2016 at 04:47:13PM -0200, Renato Westphal wrote:
>>> We shoult not call bgp_unlock() before calling
>>> bgp_delete_connected_nexthop() in the peer_free() function. Otherwise,
>>> if bgp->lock reaches zero, bgp_free() is called and peer->bgp becomes
>>> an invalid pointer in the bgp_delete_connected_nexthop() function.
>>>
>>> To fix this, move the call to bgp_unlock() to the end of peer_free().
>>>
>>> Bug exposed by commit 37d361e ("bgpd: plug several memleaks").
>>>
>>> Signed-off-by: Renato Westphal <renato at opensourcerouting.org>
>>> ---
>>>  bgpd/bgpd.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
>>> index 22d4dd8..d5aff84 100644
>>> --- a/bgpd/bgpd.c
>>> +++ b/bgpd/bgpd.c
>>> @@ -1019,8 +1019,6 @@ peer_free (struct peer *peer)
>>>  {
>>>    assert (peer->status == Deleted);
>>>
>>> -  bgp_unlock(peer->bgp);
>>> -
>>>    /* this /ought/ to have been done already through bgp_stop earlier,
>>>     * but just to be sure..
>>>     */
>>> @@ -1092,6 +1090,8 @@ peer_free (struct peer *peer)
>>>
>>>    bfd_info_free(&(peer->bfd_info));
>>>
>>> +  bgp_unlock(peer->bgp);
>>> +
>>>    memset (peer, 0, sizeof (struct peer));
>>>
>>>    XFREE (MTYPE_BGP_PEER, peer);
>>> --
>>> 1.9.1
>>>
>>>
>>> _______________________________________________
>>> cmaster-next mailing list
>>> cmaster-next at lists.nox.tf
>>> https://lists.nox.tf/listinfo/cmaster-next
>>
>> _______________________________________________
>> cmaster-next mailing list
>> cmaster-next at lists.nox.tf
>> https://lists.nox.tf/listinfo/cmaster-next



-- 
Renato Westphal