[prev in list] [next in list] [prev in thread] [next in thread] 

List:       bird-users
Subject:    Re: [Babel-users] Babel: Possible segfault in bird unfeasible update handling code
From:       Ondrej Zajicek <santiago () crfreenet ! org>
Date:       2023-01-30 22:58:37
Message-ID: Y9hLnXrLcFtk7V6d () feanor
[Download RAW message or body]

On Mon, Jan 30, 2023 at 11:10:28PM +0100, Toke Høiland-Jørgensen via Bird-users wrote:
> Juliusz Chroboczek <jch@irif.fr> writes:
> 
> >> The problematic bit is, I think, 's' in babel_handle_update can be NULL
> >> because nothing ensures the babel_source for a particular neighbour
> >> actually exists here:
> >
> > s will be passed to babel_is_feasible, which returns true if s is null.
> > Later on, s is only used if feasible is false, in which case it cannot be
> > null.
> >
> > I agree that the code is a little too subtle for comfort.
> 
> Pish posh, there's a totally-obvious comment saying /* for feasibility */ 
> next to where 's' is assigned :P
>
> And I don't think switching babel_handle_update() to use
> babel_get_source() is a good idea either; we'd end up creating new
> source objects and leave them to be garbage collected just to improve
> readability a bit; just add a comment explaining why the deref is safe? :)

Added comment and unnecessary check (it will be likely eliminated anyways).

https://gitlab.nic.cz/labs/bird/-/commit/96d7c4679df49b34be004177b10a99210af5f141

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santiago@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."

[prev in list] [next in list] [prev in thread] [next in thread]