ibmdb / node-ibm_db

IBM DB2 and IBM Informix bindings for node
MIT License
188 stars 151 forks source link

ibm_db crashes node with OUTPUT parameter of type BINARY and Data of type Buffer #942

Closed asselin closed 9 months ago

asselin commented 10 months ago

System: Linux x86_64 (Bullseye), node v18.16.1

We have a simple stored procedure call to DB2, where one of the parameters is a BINARY output parameter. We have tried both leaving the Data as the default string type and setting it to a Buffer; in both cases, node crashes due to a double memory free. This bug report is for the case of using a Buffer type. I opened #943 for non-Buffer types (since it's a different bug).

This bug was introduced in version 3.0 of ibm_db. I confirmed this by testing different levels of ibm_db (all with version 11.5.8 of the ODBC driver). Version 2.8.1 and 2.8.2 ran under load for 24 hours each with no crashes. Versions 3.0 and 3.2.1 crashed approx every 30 minutes under load with various messages:

malloc_consolidate(): invalid chunk size

free(): invalid pointer

node[1]: ../src/cleanup_queue-inl.h:32:void node::CleanupQueue::Add(node::CleanupQueue::Callback, void*): Assertion `(insertion_info.second) == (true)' failed.

double free or corruption (!prev)

This is the DB2 query:

const query = `call DB2PROD.XQHASH(?, ?, ?, ?, ?, ?, ?)`;

The parameters are:

    const oHashedValue = {
      ParamType: AppConstants.HASH_PARAM_TYPE,
      DataType: HashDataType.BINARY,
      Data: Buffer.alloc(32),
      Length: 32,
    };
    const oErrorCode = {
      ParamType: AppConstants.HASH_PARAM_TYPE,
      DataType: HashDataType.INTEGER,
      Data: '',
    };
    const oReasonCode = {
      ParamType: AppConstants.HASH_PARAM_TYPE,
      DataType: HashDataType.INTEGER,
      Data: '',
    };

    const params = [
      1,
      _.padEnd(hashType, 10),
      value.length,
      _.padEnd(value, 100),
      oHashedValue,
      oErrorCode,
      oReasonCode,
    ];

The call to ibm_db looks like:

db.query(query, params)

The issue is in the GetBufferParam() function introduced in version 3.0. In it, it grabs the pointer from the incoming buffer, and saves it into param:

    param->buffer = (char *) Buffer::Data(value);
    param->size          = Buffer::Length(value);

Later, in GetOutputParameter(), after the query completes, it this code:

      if(sqlTypeBinary || prm.c_type == SQL_C_BINARY) {
          //str = Nan::NewOneByteString((uint8_t *) prm.buffer, prm.length).ToLocalChecked();
          //prm.buffer will be freed by Garbage collector, no need to free here.
          return scope.Escape(Nan::NewBuffer((char *)prm.buffer, prm.length).ToLocalChecked());
      }

The issue is that NewBuffer() is taking ownership of the prm.buffer pointer to free that memory, but that was set from the incoming buffer, which ibm_db doesn't own, and will be freed by the GC.

The fix is to change the code in GetBufferParam() to malloc() and make a copy of the incoming Buffer:

    // param->buffer = (char *) Buffer::Data(value);
    param->buffer        = malloc(Buffer::Length(value));
    MEMCHECK( param->buffer );
    memcpy(param->buffer, (char *)Buffer::Data(value), Buffer::Length(value));

This leaves the original Buffer alone, and lets the GC take care of freeing it whenever it wants to, and providing Nan::NewBuffer() a pointer that it can free whenever it wants to.

PR submitted with the fix.

bimalkjha commented 9 months ago

ibm_db@3.2.2 released with fix of this issue. Thanks.