FreeTDS Development Group

[Bob Kline <bkline AT rksystems.com>]

On Tue, 13 Nov 2001, Vladimir wrote:

> I have installed latest CVS release of freetds, compiled it with
> option --with-tdsver=4.2 because 7.0 not support cyrillic encoding,
> compiled PHP and tried to run simple test. It was coredumped. Last
> messages in /tmp/freetds.log were:
>
> 2001-11-13 01:32:35 inside tds_process_default_tokens() marker is e3
> 2001-11-13 01:32:35 inside tds_process_default_tokens() marker is ab
>  And that is all!
>
> I use Sun Ultra5 with Solaris 2.6, latest Apache and PHP, compiled
> from sources with gcc-2.95.3

Don't think it has anything to do with Solaris or PHP.  I was running
into the same problems on an Intel Linux box directly from the unit
tests.  I wrote Brian offering to provide him with detailed results from
failed unit tests, but I think he must be swamped with other projects at
the moment, because I haven't heard back from him.

A little digging with the debugger turns up a problem in dbutil.c, in the
function dblib_handle_info_message.  The code looks like this (stripped
down a bit to highlight the essentials):

    TDSSOCKET* tds = (TDSSOCKET *) aStruct;
    DBPROCESS* dbproc = NULL;

    if (tds && tds->parent) {
        dbproc = (DBPROCESS*)tds->parent;
    }
    if( tds->msg_info->msg_number > 0 )
    {
        if(g_dblib_msg_handler)
        {
            g_dblib_msg_handler(dbproc, tds->.....);
        }
        /* and now clean up the structure for next time */
        tds_reset_msg_info(dbproc->tds_socket);
    }

There are actually three problems.  One is that although the first "if"
test prudently checks to make sure that the TDSSOCKET pointer is not
NULL, the following "if" test and controlled block ignores the results
of that first check.  If the code is going to take the trouble to check
for a NULL pointer at all (as it should) the check should control the
entire body of code in the function, like this:

    if (tds) {
        if (tds->parent)
            dbproc = (DBPROCESS*)tds->parent;
        if (tds->msg_info->msg_number > 0) { ...
        ...
    }

The second problem is similar to the first.  The call to the function
tds_reset_msg_info near the end of the function dereferences the
DBPROCESS pointer dbproc outside the control of the test above to ensure
that this pointer will be valid.

The final problem is what's dumping core.  There is no check to ensure
that dbproc->tds_socket actually points to a valid TDSSOCKET record, and
as it turns out, that pointer is NULL (if fact the entire record pointed
to by dbproc is uninitialized).

I'll leave it to Brian and others who know this code far better than I
do to come up with a fix (I haven't done enough digging to determine,
for example, whether it's sufficient to simply bypass the call to
tds_reset_msg_info if dbproc->tds_socket is NULL; perhaps this is a
symptom of a larger problem, which seems likely if the original code
assumes it will always have a valid pointer at this spot).

-- 
Bob Kline
mailto:bkline AT rksystems.com
http://www.rksystems.com