Re: tsearch2 problem rank_cd() (possibly) crashing postgres

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Joshua D(dot) Drake" <jd(at)commandprompt(dot)com>
Cc: pgsql-admin(at)postgresql(dot)org, Oleg Bartunov <oleg(at)sai(dot)msu(dot)su>, Teodor Sigaev <teodor(at)sigaev(dot)ru>
Subject: Re: tsearch2 problem rank_cd() (possibly) crashing postgres
Date: 2006-12-08 16:12:54
Message-ID: 5964.1165594374@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-admin

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> On Fri, 2006-12-08 at 10:47 -0500, Tom Lane wrote:
>> Nonetheless, dumping core on bad input is not acceptable behavior ...

> Is it time to require test cases for contrib modules?

tsearch2 *has* a regression test. ATM it sounds like the problem is
that the OP tried to use a new library with old pg_proc entries that
defined different parameter sets for the same-named C functions; a
situation that no regression test would have exercised anyway.

That change was probably unwise on Oleg and Teodor's part, but what's
done is done. My point is that now that we know the failure mode, we
need to add some defenses. The OP is certainly not the last DBA who
will make this mistake during 8.1->8.2 upgrade.

It looks to me like the problem is that with the old pg_proc entries,
an "int4" will get passed where the code is expecting "float4[]",
so it tries to dereference the int and crashes. There doesn't seem
to be any real cheap defense --- we might have to add a
get_fn_expr_argtype() call into rank_cd, and anything else that's been
changed similarly. But I don't think this is negotiable. Backend
crashes are bad.

regards, tom lane

In response to

Responses

Browse pgsql-admin by date

  From Date Subject
Next Message Joshua D. Drake 2006-12-08 16:18:42 Re: tsearch2 problem rank_cd() (possibly) crashing postgres
Previous Message Oleg Bartunov 2006-12-08 16:11:56 Re: tsearch2 problem rank_cd() (possibly) crashing postgres