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

Donald Sharp sharpd at cumulusnetworks.com
Thu Dec 1 10:47:28 EST 2016


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




More information about the dev mailing list