BUG in obex_data_indication()

2005-07-02
2013-05-01
  • Andy Neverowsy
    Andy Neverowsy
    2005-07-02

    I have Siemens C65 and try patch multisync to work through data cable. During writing this patch I found bug in libopenobex-1.0.1.
    Sometimes reading from cableport get only 1 byte at once.  obex_data_indication in obex_main.c can read any bytes at once, but there is a bug.
    Code:
    int obex_data_indication(obex_t *self, uint8_t *buf, int buflen) {
    ...
    if(msg->len < 3)  {
      actual = obex_transport_read(self, 3 - (msg->len), buf, buflen);
    ...
                    buf += actual;
                    buflen -= actual;
                    g_netbuf_put(msg, actual);
    }
    if(msg->len >= 3) {
       hdr = (obex_common_hdr_t *) msg->data;
       size = ntohs(hdr->len);

       actual = 0;
        if(msg->len != (int) ntohs(hdr->len)) {
           actual = obex_transport_read(self, size - msg->len, buf,
                                    buflen);
            /* Check if we are still connected */
            if (actual <= 0) {
                obex_deliver_event(self, OBEX_EV_LINKERR, 0, 0, TRUE);
                 return actual;
              }
          }
    }
    ..
    }

    We read from device and call OBEX_CustomDataFeed, that call obex_data_indication.
    We read first byte. msg->len ==0 (< 3).  Function write the byte to internal buffer (through call obex_transport_read) and return. All right.
    The same with second byte.
    We read THIRD byte.
    msg->len ==2 (< 3).  Write byte as usualy. buflen became 0 (buflen -= actual;). msg->len became 3. All right.
    Because (msg->len >= 3) we calc packet size and call
    obex_transport_read again. But now bufflen == 0 and  obex_transport_read return 0. actual=0 and we /* Check if we are still connected */. During this check connection will be closed! But we don't neeed check in this case because we know that received this THIRD byte!
    Code must look like this:

    int obex_data_indication(obex_t *self, uint8_t *buf, int buflen) {
    ...
    int oldbuflen = buflen;
    ...
          /* Check if we are still connected */
          if ((actual <= 0) && (oldbuflen <= 0))  {
    ...
    }

    P.S.
    I have not access to developer forum. And wrote to this one.