[Xorp-hackers] Potential null pointer dereference.

[Kristian Larsson]

On Wed, Feb 27, 2008 at 02:59:58AM -0800, Pavlin Radoslavov wrote:
> Ben Greear <greearb at candelatech.com> wrote:
> 
> > Ben Greear wrote:
> > > While merging my old patch set with the latest xorp tree, I believe
> > > I found a potential null pointer dereference.  Here is my attempt
> > > at fixing it:
> > > 
> > > [greearb at file-server control_socket]$ cvs diff -u netlink_socket_utilities.cc
> > > Index: netlink_socket_utilities.cc
> > > ===================================================================
> > > RCS file: /cvs/xorp/fea/data_plane/control_socket/netlink_socket_utilities.cc,v
> > > retrieving revision 1.12
> > > diff -u -r1.12 netlink_socket_utilities.cc
> > > --- netlink_socket_utilities.cc 8 Jan 2008 23:30:09 -0000       1.12
> > > +++ netlink_socket_utilities.cc 26 Feb 2008 19:40:36 -0000
> > > @@ -332,9 +332,10 @@
> > >          const IfTreeVif* vifp = iftree.find_vif(if_index);
> > >          if (vifp == NULL) {
> > >              if (! is_deleted) {
> > > -               XLOG_FATAL("Could not find interface and vif for index %d",
> > > +               XLOG_ERROR("Could not find interface and vif for index %d",
> > >                             if_index);
> > >              }
> > > +           return XORP_ERROR;
> > >          }
> > >          if_name = vifp->ifname();
> > >          vif_name = vifp->vifname();
> > > 
> > 
> > 
> > Here's another one:
> > [greearb at file-server ifconfig]$ cvs diff -u ifconfig_parse_netlink_socket.cc
> > Index: ifconfig_parse_netlink_socket.cc
> > ===================================================================
> > RCS file: /cvs/xorp/fea/data_plane/ifconfig/ifconfig_parse_netlink_socket.cc,v
> > retrieving revision 1.17
> > diff -u -r1.17 ifconfig_parse_netlink_socket.cc
> > --- ifconfig_parse_netlink_socket.cc    21 Feb 2008 02:02:33 -0000      1.17
> > +++ ifconfig_parse_netlink_socket.cc    26 Feb 2008 20:06:18 -0000
> > @@ -603,7 +603,8 @@
> >              //
> >              return;
> >          }
> > -       XLOG_FATAL("Could not find vif with index %u in IfTree", if_index);
> > +       XLOG_ERROR("Could not find vif with index %u in IfTree", if_index);
> > +        return;
> >       }
> >       debug_msg("Address event on interface %s vif %s with interface index %u\n",
> >                vifp->ifname().c_str(), vifp->vifname().c_str(),
> > 
> 
> Ben,
> 
> Did you see those FATAL/ERROR statements actually triggered when
> running XORP?
> The reason those XLOG statements are FATAL is to capture bugs that
> might be hiding somewhere else.
> If you were able to trigger those statements, could you provide
> instructions how to reproduce the problem so we can investigate it.

For a true lab scenario this would probably be
the wanted behaviour, but for software that is
supposed to be able to work in the real world it
is probably not.

One of the most interesting parts I've seen with
IOS XR (Ciscos new operating system for the CRS-1)
is the ability to track what all processes are
doing all the time, so if a fault occurs, debug
information is saved at all times, even though the
administrator of the system has not asked of this.

This is very Service Providerisch ;), it'd be nice
to see XORP have something like it.

  -K

-- 
Kristian Larsson                                        KLL-RIPE
Network Engineer & Peering Coordinator      SpriteLink [AS39525]
+46 704 910401			              kll at spritelink.net