Обсуждение: Ask for two questions on psqlodbc

Hi,

(2014/07/02 18:09), cobainpluto wrote:
> Dear all，
> Recently, I used Static Code Analyzer(Fortify) to analyze
> psqlodbc-09.03.0300 codes, and found two potential Memory Leak
> problems in qresult.c file.
>
> Details are as follows :
> 1.Potential Memory Leak problem
> qresult.c:962: in QR_next_tuple()
> 962 mres = CC_send_query(conn, movecmd, NULL, 0, stmt);
> There is a dynamically allocated memory in CC_send_query_append(...).
> If follow the below path, from here to RETURN (-1), the applied memory
> space is not free, so it is possiblehas to generate Memory
> Leak.
> ---------------------------------------------------------------
> qresult.c:963 - BranchNotTaken : Branch not taken: (mres != 0)
> qresult.c:971 - BranchTaken : Branch taken: (sscanf(mres->command, "MOVE
> %lu", (&moved)) > 0)
> qresult.c:974 - BranchTaken : Branch taken: (moved < movement)
> qresult.c:993 - BranchTaken : Branch taken: (2 == self->move_direction)
> qresult.c:998 - BranchTaken : Branch taken: (getNthValid(self, (<inline
> expression> - 1), 4, self->move_offset, (&backpt)) < 0)
> qresult.c:1004 - EndScope : RETURN(-1)

It seems a memory leak.
I would fix it.

> ---------------------------------------------------------------
>
> 2、Potential Null Dereference problem
> qresult.c:1691: in QR_read_a_tuple_from_db()
> 1691 &this_keyset->blocknum, &this_keyset->offset);
> qresult.c:1693: in QR_read_a_tuple_from_db()
> 1693 this_keyset->oid = strtoul(buffer, NULL, 10);
> Here reference to the this_keyset.
> If follow the below path,value of this_keyset is always NULL before
> referring to this_keyset, so it is possiblehas to generate Null
> Dereference possible.
> ---------------------------------------------------------------
> qresult.c:1571 - Assigned null : KeySet *this_keyset = NULL;
> qresult.c:1590 - BranchNotTaken : Branch not taken: (0 == (self->flags & 1))
> qresult.c:1624 - BranchTaken : Branch taken: (field_lf < ci_num_fields)
> qresult.c:1668 - BranchNotTaken : Branch not taken: (isnull == 0)
> qresult.c:1676 - BranchTaken : Branch taken: (field_lf >= effective_cols)
> qresult.c:1687 - BranchTaken : Branch taken: (field_lf >= effective_cols)

Though I'm suspcious if it could occur, I would check it.

Thanks.
Hiroshi Inoue

(2014/07/07 22:37), cobainpluto wrote:
> Dear,
>
> Could you please tell me when will you fix it?

I already fixed the first one.
Please look at
 http://git.postgresql.org/gitweb/?p=psqlodbc.git;a=commit;h=86198069efe4cffed05eecf8669c5772334d273d
.
As for the 2nd one, I don't understand how it occurs.
Anyway it may be safe to add some check.

regards,
Hiroshi Inoue

> Thanks!
>
> regards,
> Pluto Cobain>
>
>  > Date: Thu, 3 Jul 2014 23:35:33 +0900
>  > From: inoue@tpf.co.jp
>  > To: pluto_cbin@outlook.com; pgsql-odbc@postgresql.org
>  > Subject: Re: [ODBC] Ask for two questions on psqlodbc
>  >
>  > Hi,
>  >
>  > (2014/07/02 18:09), cobainpluto wrote:
>  > > Dear all，
>  > > Recently, I used Static Code Analyzer(Fortify) to analyze
>  > > psqlodbc-09.03.0300 codes, and found two potential Memory Leak
>  > > problems in qresult.c file.
>  > >
>  > > Details are as follows :
>  > > 1.Potential Memory Leak problem
>  > > qresult.c:962: in QR_next_tuple()
>  > > 962 mres = CC_send_query(conn, movecmd, NULL, 0, stmt);
>  > > There is a dynamically allocated memory in CC_send_query_append(...).
>  > > If follow the below path, from here to RETURN (-1), the applied memory
>  > > space is not free, so it is possiblehas to generate Memory
>  > > Leak.
>  > > ---------------------------------------------------------------
>  > > qresult.c:963 - BranchNotTaken : Branch not taken: (mres != 0)
>  > > qresult.c:971 - BranchTaken : Branch taken: (sscanf(mres->command,
> "MOVE
>  > > %lu", (&moved)) > 0)
>  > > qresult.c:974 - BranchTaken : Branch taken: (moved < movement)
>  > > qresult.c:993 - BranchTaken : Branch taken: (2 == self->move_direction)
>  > > qresult.c:998 - BranchTaken : Branch taken: (getNthValid(self,
> (<inline
>  > > expression> - 1), 4, self->move_offset, (&backpt)) < 0)
>  > > qresult.c:1004 - EndScope : RETURN(-1)
>  >
>  > It seems a memory leak.
>  > I would fix it.
>  >
>  > > ---------------------------------------------------------------
>  > >
>  > > 2、Potential Null Dereference problem
>  > > qresult.c:1691: in QR_read_a_tuple_from_db()
>  > > 1691 &this_keyset->blocknum, &this_keyset->offset);
>  > > qresult.c:1693: in QR_read_a_tuple_from_db()
>  > > 1693 this_keyset->oid = strtoul(buffer, NULL, 10);
>  > > Here reference to the this_keyset.
>  > > If follow the below path,value of this_keyset is always NULL before
>  > > referring to this_keyset, so it is possiblehas to generate Null
>  > > Dereference possible.
>  > > ---------------------------------------------------------------
>  > > qresult.c:1571 - Assigned null : KeySet *this_keyset = NULL;
>  > > qresult.c:1590 - BranchNotTaken : Branch not taken: (0 ==
> (self->flags & 1))
>  > > qresult.c:1624 - BranchTaken : Branch taken: (field_lf < ci_num_fields)
>  > > qresult.c:1668 - BranchNotTaken : Branch not taken: (isnull == 0)
>  > > qresult.c:1676 - BranchTaken : Branch taken: (field_lf >=
> effective_cols)
>  > > qresult.c:1687 - BranchTaken : Branch taken: (field_lf >=
> effective_cols)
>  >
>  > Though I'm suspcious if it could occur, I would check it.
>  >
>  > Thanks.
>  > Hiroshi Inoue
>  >
>  >
>  > --
>  > Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
>  > To make changes to your subscription:
>  > http://www.postgresql.org/mailpref/pgsql-odbc


--
I am using the free version of SPAMfighter.
SPAMfighter has removed 11445 of my spam emails to date.
Get the free SPAMfighter here: http://www.spamfighter.com/len

Do you have a slow PC? Try a Free scan
http://www.spamfighter.com/SLOW-PCfighter?cid=sigen

On 07/09/2014 10:03 PM, cobainpluto wrote:
> I want to know if you will add some check and when?

You're aware that psqlODBC, like PostgreSQL its self, is largely a
_volunteer_ effort, right? Or at least, that when work is done on a paid
basis it's usually for customers contracting specific work who have
specific requirements?

Hiroshi has been doing a great deal of work on psqlODBC and seems to get
very little by way of thanks for it.

It's helpful to do things like run static analysis tools, but the
greater part of the work involved in that is determining whether the
results are valid issues or false positives - and if they're valid, how
best to fix them. So when you run a tool then ask someone else to
process the results, that's putting a lot of work on them.

It would be extremely helpful if you were to spend some time examining
the code paths identified by this tool and forming opinions about what
might be happening / how the error could arise. If you can turn the tool
output into an explanation of "here's a bug and here's how it happens"
that makes fixing it a _lot_ quicker and easier.

It's good to see people interested and trying to contribute, so I'm not
trying to push back. I'm just trying to explain an aspect of this kind
of work that you might not be aware of. I hope it is helpful.

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services