Hi On Sat, Sep 13, 2014 at 11:24 AM, <[email protected]> wrote: > From: Philippe De Swert <[email protected]> > > hashmap_get can return null, so as we dereference it > immediately after calling it, we could crash. > It is unlikely to occur though I expect. I am however unsure > what error should be reported (if at all). > > Coverity CID#1237656 > --- > src/resolve/resolved-manager.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c > index f979897..ba175e6 100644 > --- a/src/resolve/resolved-manager.c > +++ b/src/resolve/resolved-manager.c > @@ -1695,6 +1695,8 @@ int manager_ifindex_is_loopback(Manager *m, int > ifindex) { > return -EINVAL; > > l = hashmap_get(m->links, INT_TO_PTR(ifindex)); > + if(!l) > + return -EINVAL;
Ok, this can really be triggered if we receive messages on a link that got hotplugged. This is unlikely, but I'm not sure treating such links as loopback is the way to go. Maybe Tom has an idea what to do? The callers of manager_ifindex_is_loopback() treat errors as 1, thus as loopback device. They lookup the device via address-match then, so maybe this is fine. But I feel uncomfortable applying this without mentioning the race anywhere. Calling into RTNL to read the link is probably overkill. We could set the RTNL event priority higher than the I/O socket, so the RTNL socket is guaranteed to be read prior to any packet. That's still not a guarantee, though. And maybe I care way too much for such a tiny tiny race.. who knows.. I hope Tom can apply this. Thanks David > if (l->flags & IFF_LOOPBACK) > return 1; > > -- > 1.8.3.2 > > _______________________________________________ > systemd-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/systemd-devel _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
