Re: [HACKERS] Possible pointer dereference
While at it the assert(cnfa != NULL cnfa-nstates != 0); at src/backend/regex/rege_dfa.c:282 is issued too late indeed at line 278 and 279 cnfa was already dereferenced. Same for assert(t != NULL) in src/backend/regex/regexec.c:821 is issued way too late. On Thu, 28 May 2015 at 15:59 Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, May 27, 2015 at 8:57 PM, Haribabu Kommi kommi.harib...@gmail.com wrote: By correcting the following way will solve the problem. return ts ? (*ts != 0) : false; instead of retun *ts != 0; Attached a patch for it. If the only caller always passes a valid pointer, there's no point in adding this check. We have many functions in our source base that assume that the caller will pass a valid pointer, and changing them all would make the code bigger, harder to read, and possibly slower, without any real benefit. Well, we should either install something like Haribabu's patch, or else remove the existing tests in the function that allow ts to be NULL. And the function's API contract comment needs to be clarified in either case; the real bug here is lack of a specification. I don't particularly have an opinion on whether it's valuable to allow this function to be called without receiving a timestamp back. Perhaps the authors of the patch can comment on that. regards, tom lane
Re: [HACKERS] Possible pointer dereference
Robert Haas robertmh...@gmail.com writes: On Wed, May 27, 2015 at 8:57 PM, Haribabu Kommi kommi.harib...@gmail.com wrote: By correcting the following way will solve the problem. return ts ? (*ts != 0) : false; instead of retun *ts != 0; Attached a patch for it. If the only caller always passes a valid pointer, there's no point in adding this check. We have many functions in our source base that assume that the caller will pass a valid pointer, and changing them all would make the code bigger, harder to read, and possibly slower, without any real benefit. Well, we should either install something like Haribabu's patch, or else remove the existing tests in the function that allow ts to be NULL. And the function's API contract comment needs to be clarified in either case; the real bug here is lack of a specification. I don't particularly have an opinion on whether it's valuable to allow this function to be called without receiving a timestamp back. Perhaps the authors of the patch can comment on that. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Possible pointer dereference
I'm playing with a static analyzer and it's giving out some real error analyzing postgresql code base like the following one src/backend/access/transam/commit_ts.c return *ts != 0 // line 321 but a few line up (line 315) ts is checked for null, so either is not needed to check for null or *ts can lead to a null pointer dereference. Same happens a few line later lines 333 and 339 Regards Gaetano
Re: [HACKERS] Possible pointer dereference
On Thu, May 28, 2015 at 6:07 AM, Gaetano Mendola mend...@gmail.com wrote: I'm playing with a static analyzer and it's giving out some real error analyzing postgresql code base like the following one src/backend/access/transam/commit_ts.c return *ts != 0 // line 321 but a few line up (line 315) ts is checked for null, so either is not needed to check for null or *ts can lead to a null pointer dereference. Same happens a few line later lines 333 and 339 Thanks for providing detailed information. The function TransactionIdGetCommitTsData is currently used only at one place. The caller always passes an valid pointer to this function. So there shouldn't be a problem. But in future if the same function is used at somewhere by passing the NULL pointer then it leads to a crash. By correcting the following way will solve the problem. return ts ? (*ts != 0) : false; instead of retun *ts != 0; Attached a patch for it. Regards, Hari Babu Fujitsu Australia commit_ts_fix.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
