[ovs-dev] openflow-1.0 : ovs-vswitchd received SIGSEGV when netflow was specified

Jesse Gross jesse at nicira.com
Thu Feb 25 14:18:55 UTC 2010


Thanks for pointing this out.  However, I think that it would be better to
do the check in the collectors library since it is shared with sFlow, which
could have the same problem.  I pushed the following patch to the 'next'
branch (which openflow-1.0 was merged into):

commit 7e56c85c02c547deda93ec09a589eae7e253fc58
Author: Jesse Gross <jesse at nicira.com>
Date:   Thu Feb 25 09:02:26 2010 -0500

    collectors: Check for NULL set of collectors.

    If the set of collectors for NetFlow or sFlow is empty due to
    an error it will cause a crash when trying to send data.

    Reported-by: Tetsuo NAKAGAWA <nakagawa at mxc.nes.nec.co.jp>

diff --git a/ofproto/collectors.c b/ofproto/collectors.c
index 4589f32..f063983 100644
--- a/ofproto/collectors.c
+++ b/ofproto/collectors.c
@@ -111,13 +111,15 @@ collectors_destroy(struct collectors *c)
 void
 collectors_send(const struct collectors *c, const void *payload, size_t n)
 {
-    size_t i;
+    if (c) {
+        size_t i;

-    for (i = 0; i < c->n_fds; i++) {
-        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-        if (send(c->fds[i], payload, n, 0) == -1) {
-            VLOG_WARN_RL(&rl, "sending to collector failed: %s",
-                         strerror(errno));
+        for (i = 0; i < c->n_fds; i++) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            if (send(c->fds[i], payload, n, 0) == -1) {
+                VLOG_WARN_RL(&rl, "sending to collector failed: %s",
+                             strerror(errno));
+            }
         }
     }
 }
@@ -125,5 +127,5 @@ collectors_send(const struct collectors *c, const void
*payl
 int
 collectors_count(const struct collectors *c)
 {
-    return c->n_fds;
+    return c ? c->n_fds : 0;
 }


On Wed, Feb 24, 2010 at 11:16 PM, Tetsuo NAKAGAWA <
nakagawa at mxc.nes.nec.co.jp> wrote:

> Hi.
>
> I'm using Open vSwitch with openflow-1.0 branch.
>
> When I specified "netflow.<name>.host" in ovs-vswitchd.conf,
> ovs-vswitchd received SIGSEGV.
>
>  Feb 24 21:22:24|00012|collectors|WARN|couldn't open connection to
> collector 192.168.100.20:9901 (Network is unreachable)
>  Feb 24 21:22:34|00013|fault|EMER|Caught signal 11.
>
> The investigation result by gdb is as follows.
>
>  Program terminated with signal 11, Segmentation fault.
>  [New process 6903]
>  #0  log_backtrace () at lib/fault.c:51
>  51               frame != NULL && frame[0] != NULL;
>  (gdb) bt
>  #0  log_backtrace () at lib/fault.c:51
>  #1  0x0807586f in fault_handler (sig_nr=11) at lib/fault.c:34
>  #2  <signal handler called>
>  #3  collectors_send (c=0x0, payload=0x86cdea0, n=552) at
> ofproto/collectors.c:116
>  #4  0x08069577 in netflow_run (nf=0x86cc1a0) at ofproto/netflow.c:194
>  #5  0x080654c8 in ofproto_run1 (p=0x869ed20) at ofproto/ofproto.c:889
>  #6  0x0805554c in bridge_run () at vswitchd/bridge.c:1162
>  #7  0x0805e787 in main (argc=Cannot access memory at address 0x0
>  ) at vswitchd/ovs-vswitchd.c:102
>  #3  collectors_send (c=0x0, payload=0x86cdea0, n=552) at
> ofproto/collectors.c:116
>  116         for (i = 0; i < c->n_fds; i++) {
>  (gdb) p c
>  $1 = (const struct collectors *) 0x0
>
> SIGSEGV occurs becaus 'c' (collecturs) is NULL.
>
>  (gdb) l
>  111     void
>  112     collectors_send(const struct collectors *c, const void *payload,
> size_t n)
>  113     {
>  114         size_t i;
>  115
>  116         for (i = 0; i < c->n_fds; i++) {
>  117             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1,
> 5);
>  118             if (send(c->fds[i], payload, n, 0) == -1) {
>  119                 VLOG_WARN_RL(&rl, "sending to collector failed: %s",
>  120                              strerror(errno));
>  (gdb) frame 4
>  #4  0x08069577 in netflow_run (nf=0x86cc1a0) at ofproto/netflow.c:194
>  194             collectors_send(nf->collectors, nf->packet.data,
> nf->packet.size);
>  (gdb) l
>  189
>  190     void
>  191     netflow_run(struct netflow *nf)
>  192     {
>  193         if (nf->packet.size) {
>  194             collectors_send(nf->collectors, nf->packet.data,
> nf->packet.size);
>  195             nf->packet.size = 0;
>  196         }
>  197     }
>  198
>  (gdb) p *nf
>  $2 = {engine_type = 0 '\0', engine_id = 0 '\0', boot_time = 1266990334236,
> collectors = 0x0, add_id_to_iface = false, netflow_cnt = 52, packet = {base
> = 0x86cdea0,
>      allocated = 1500, data = 0x86cdea0, size = 552, l2 = 0x0, l3 = 0x0, l4
> = 0x0, l7 = 0x0, next = 0x0, private = 0x0}, active_timeout = 600000,
>    reconfig_time = 1266990334236}
>
> "ofproto->netflow->collectors" is NULL.
>
> "collectors" was set NULL because inet_open_active() was
> failed that is called by collectors_create().
>
> There were no defect in previous source code because
> "struct collectors" doesn't exist.
>
> The previous source code is as follows.
>
>  void
>  netflow_run(struct netflow *nf)
>  {
>      size_t i;
>
>      if (!nf->packet.size) {
>          return;
>      }
>
>      for (i = 0; i < nf->n_fds; i++) {
>          if (send(nf->fds[i], nf->packet.data, nf->packet.size, 0) == -1) {
>              VLOG_WARN_RL(&rl, "netflow message send failed: %s",
>                           strerror(errno));
>          }
>      }
>      nf->packet.size = 0;
>  }
>
> When inet_open_active() was failed, nf->n_fds will set 0.
> Therefore nothing is processed and a problem doesn't occur.
>
> But "collectors" has added to the present source code by
> following correction.
>
>
> http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commit;h=6bab37989b1b5e8981bee80e1fedef621799332c
>
> Therefore before referring to
> "ofproto->netflow->collectors->n_fds", a check of whether
> "ofproto->netflow->collectors" is NULL is needed.
>
> The patch to this defect is as follows.
>
>  --- ./openvswitch-openflow-1.0-20100216.org/ofproto/netflow.c
> 2010-02-13 06:18:45.000000000 +0900
>  +++ ./openvswitch-openflow-1.0-20100216/ofproto/netflow.c       2010-02-25
> 10:48:16.000000000 +0900
>  @@ -190,7 +190,7 @@
>   void
>   netflow_run(struct netflow *nf)
>   {
>  -    if (nf->packet.size) {
>  +    if (nf->packet.size && nf->collectors) {
>           collectors_send(nf->collectors, nf->packet.data,
> nf->packet.size);
>           nf->packet.size = 0;
>       }
>
>
> --- Tetsuo NAKAGAWA
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100225/16938b14/attachment-0003.html>


More information about the dev mailing list