ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-624

The C Client cause core dump when receive error data from Zookeeper Server

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.2.0
    • Fix Version/s: 3.3.0
    • Component/s: c client
    • Labels:
      None
    • Environment:

      Linux 2.6.9 x86_64

    • Hadoop Flags:
      Reviewed
    • Tags:
      c client, core dump

      Description

      I encountered a problem today that the Zookeeper C Client (version 3.2.0) core dump when reconnected and did some operations on the zookeeper server which just restarted. The gdb infomation is like:

      (gdb) bt
      #0 0x000000302af71900 in memcpy () from /lib64/tls/libc.so.6
      #1 0x000000000047bfe4 in ia_deserialize_string (ia=Variable "ia" is not available.) at src/recordio.c:270
      #2 0x000000000047ed20 in deserialize_CreateResponse (in=0x9cd870, tag=0x50a74e "reply", v=0x409ffe70) at generated/zookeeper.jute.c:679
      #3 0x000000000047a1d0 in zookeeper_process (zh=0x9c8c70, events=Variable "events" is not available.) at src/zookeeper.c:1895
      #4 0x00000000004815e6 in do_io (v=Variable "v" is not available.) at src/mt_adaptor.c:310
      #5 0x000000302b80610a in start_thread () from /lib64/tls/libpthread.so.0
      #6 0x000000302afc6003 in clone () from /lib64/tls/libc.so.6
      #7 0x0000000000000000 in ?? ()
      (gdb) f 1
      #1 0x000000000047bfe4 in ia_deserialize_string (ia=Variable "ia" is not available.) at src/recordio.c:270
      270 in src/recordio.c
      (gdb) info locals
      priv = (struct buff_struct *) 0x9cd8d0
      len = -1
      rc = Variable "rc" is not available.

      According to the source code,
      int ia_deserialize_string(struct iarchive *ia, const char *name, char **s)
      {
      struct buff_struct *priv = ia->priv;
      int32_t len;
      int rc = ia_deserialize_int(ia, "len", &len);
      if (rc < 0)
      return rc;
      if ((priv->len - priv->off) < len)

      { return -E2BIG; }

      *s = malloc(len+1);
      if (!*s)

      { return -ENOMEM; }

      memcpy(*s, priv->buffer+priv->off, len);
      (*s)[len] = '\0';
      priv->off += len;
      return 0;
      }

      the variable len is set by ia_deserialize_int, and the returned len doesn't been checked, so the client segment fault when trying to memcpy -1 byte data.
      In the source file recordio.c, there are many functions which don't check the returned len. They all might cause segment fault in some kind of situations.

      1. ZOOKEEPER-624.patch
        2 kB
        Mahadev konar
      2. ZOOKEEPER-624.patch
        2 kB
        Mahadev konar

        Activity

        Hide
        Mahadev konar added a comment -

        marking this for 3.3 for now. we should find out if this is critical or not.

        Show
        Mahadev konar added a comment - marking this for 3.3 for now. we should find out if this is critical or not.
        Hide
        Patrick Hunt added a comment -

        Ben, can you have a look at this one as well?

        Show
        Patrick Hunt added a comment - Ben, can you have a look at this one as well?
        Hide
        Mahadev konar added a comment -

        this patch checks for len in deserialize_String() and also adds a testcase. I checked for other places but looks like everywhere else we do check for len being -1.

        Show
        Mahadev konar added a comment - this patch checks for len in deserialize_String() and also adds a testcase. I checked for other places but looks like everywhere else we do check for len being -1.
        Hide
        Mahadev konar added a comment -

        minor change to the patch with check for len <0.

        Show
        Mahadev konar added a comment - minor change to the patch with check for len <0.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12438363/ZOOKEEPER-624.patch
        against trunk revision 921201.

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/7/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/7/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/7/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12438363/ZOOKEEPER-624.patch against trunk revision 921201. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/7/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/7/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/7/console This message is automatically generated.
        Hide
        Benjamin Reed added a comment -

        +1 good job mahadev!

        Show
        Benjamin Reed added a comment - +1 good job mahadev!
        Hide
        Mahadev konar added a comment -

        I just committed this.

        Show
        Mahadev konar added a comment - I just committed this.
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #752 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/752/)
        . The C Client cause core dump when receive error data from Zookeeper Server (mahadev)

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #752 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/752/ ) . The C Client cause core dump when receive error data from Zookeeper Server (mahadev)

          People

          • Assignee:
            Mahadev konar
            Reporter:
            Qian Ye
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development