Tom Lane [Wed, 3 Mar 2010 20:31:22 +0000 (20:31 +0000)]
Fix a couple of places that would loop forever if attempts to read a stdio file
set ferror() but never set feof(). This is known to be the case for recent
glibc when trying to read a directory as a file, and might be true for other
platforms/cases too. Per report from Ed L. (There is more that we ought to
do about his report, but this is one easily identifiable issue.)
Tom Lane [Wed, 3 Mar 2010 19:10:35 +0000 (19:10 +0000)]
Make contrib/xml2 use core xml.c's error handler, when available (that is,
in versions >= 8.3). The core code is more robust and efficient than what
was there before, and this also reduces risks involved in swapping different
libxml error handler settings.
Before 8.3, there is still some risk of problems if add-on modules such as
Perl invoke libxml without setting their own error handler. Given the lack
of reports I'm not sure there's a risk in practice, so I didn't take the
step of actually duplicating the core code into older contrib/xml2 branches.
Instead I just tweaked the existing code to ensure it didn't leave a dangling
pointer to short-lived memory when throwing an error.
Tom Lane [Wed, 3 Mar 2010 17:30:01 +0000 (17:30 +0000)]
Export xml.c's libxml-error-handling support so that contrib/xml2 can use it
too, instead of duplicating the functionality (badly).
I renamed xml_init to pg_xml_init, because the former seemed just a bit too
generic to be safe as a global symbol. I considered likewise renaming
xml_ereport to pg_xml_ereport, but felt that the reference to ereport probably
made it sufficiently PG-centric already.
Fix numericlocale psql option when used with a null string and latex and troff
formats; a null string must not be formatted as a numeric. The more exotic
formats latex and troff also incorrectly formatted all strings as numerics
when numericlocale was on.
Backpatch to 8.1 where numericlocale option was added.
Tom Lane [Mon, 1 Mar 2010 18:08:16 +0000 (18:08 +0000)]
Fix contrib/xml2 so regression test still works when it's built without libxslt.
This involves modifying the module to have a stable ABI, that is, the
xslt_process() function still exists even without libxslt. It throws a
runtime error if called, but doesn't prevent executing the CREATE FUNCTION
call. This is a good thing anyway to simplify cross-version upgrades.
Tom Lane [Mon, 1 Mar 2010 05:16:48 +0000 (05:16 +0000)]
Remove xmlCleanupParser calls from contrib/xml2.
These are unnecessary and probably dangerous. I don't see any immediate
risk situations in the core XML support or contrib/xml2 itself, but there
could be issues with external uses of libxml2, and in any case it's an
accident waiting to happen.
Tom Lane [Mon, 1 Mar 2010 03:41:11 +0000 (03:41 +0000)]
Back-patch today's memory management fixups in contrib/xml2.
Prior to 8.3, these changes are not critical for compatibility with core
Postgres, since core had no libxml2 calls then. However there is still
a risk if contrib/xml2 is used along with libxml2 functionality in Perl
or other loadable modules. So back-patch to all versions.
Also back-patch addition of regression tests. I'm not sure how many of
the cases are interesting without the interaction with core xml code,
but a silly regression test is still better than none at all.
Tom Lane [Mon, 1 Mar 2010 02:21:40 +0000 (02:21 +0000)]
Back-patch changes of 2009-05-13 in xml.c's memory management.
I was afraid to do this when these changes were first made, but now that
8.4 has seen some field use it should be all right to back-patch. These
changes are really quite necessary in order to give xml.c any hope of
co-existing with loadable modules that also wish to use libxml2.
Tom Lane [Thu, 25 Feb 2010 21:00:10 +0000 (21:00 +0000)]
Allow predicate_refuted_by() to deduce that NOT A refutes A.
We had originally made the stronger assumption that NOT A refutes any B
if B implies A, but this fails in three-valued logic, because we need to
prove B is false not just that it's not true. However the logic does
go through if B is equal to A.
Recognizing this limited case is enough to handle examples that arise when
we have simplified "bool_var = true" or "bool_var = false" to just "bool_var"
or "NOT bool_var". If we had not done that simplification then the
btree-operator proof logic would have been able to prove that the expressions
were contradictory, but only for identical expressions being compared to the
constants; so handling identical A and B covers all the same cases.
The motivation for doing this is to avoid unexpected asymmetrical behavior
when a partitioned table uses a boolean partitioning column, as in today's
gripe from Dominik Sander.
Back-patch to 8.2, which is as far back as predicate_refuted_by attempts to
do anything at all with NOTs.
Magnus Hagander [Thu, 25 Feb 2010 13:26:23 +0000 (13:26 +0000)]
Add configuration parameter ssl_renegotiation_limit to control
how often we do SSL session key renegotiation. Can be set to
0 to disable renegotiation completely, which is required if
a broken SSL library is used (broken patches to CVE-2009-3555
a known cause) or when using a client library that can't do
renegotiation.
Tom Lane [Wed, 24 Feb 2010 18:02:36 +0000 (18:02 +0000)]
Allow zero-dimensional (ie, empty) arrays in contrib/ltree operations.
The main motivation for changing this is bug #4921, in which it's pointed out
that it's no longer safe to apply ltree operations to the result of
ARRAY(SELECT ...) if the sub-select might return no rows. Before 8.3,
the ARRAY() construct would return NULL, which might or might not be helpful
but at least it wouldn't result in an error. Now it returns an empty array
which results in a failure for no good reason, since the ltree operations
are all perfectly capable of dealing with zero-element arrays.
As far as I can find, these ltree functions are the only places where zero
array dimensionality is rejected unnecessarily.
Back-patch to 8.3 to prevent behavioral regression of queries that worked
in older releases.
Itagaki Takahiro [Fri, 19 Feb 2010 01:06:51 +0000 (01:06 +0000)]
Fix STOP WAL LOCATION in backup history files no to return the next
segment of XLOG_BACKUP_END record even if the the record is placed
at a segment boundary. Furthermore the previous implementation could
return nonexistent segment file name when the boundary is in segments
that has "FE" suffix; We never use segments with "FF" suffix.
Backpatch to 8.0, where hot backup was introduced.
Tom Lane [Thu, 18 Feb 2010 23:50:20 +0000 (23:50 +0000)]
Volatile-ize all five places where we expect a PG_TRY block to restore
old memory context in plpython. Before only one of them was marked
volatile, but per report from Zdenek Kotala, some compilers do the
wrong thing here.
Tom Lane [Thu, 18 Feb 2010 18:42:04 +0000 (18:42 +0000)]
Fix ExecEvalArrayRef to pass down the old value of the array element or slice
being assigned to, in case the expression to be assigned is a FieldStore that
would need to modify that value. The need for this was foreseen some time
ago, but not implemented then because we did not have arrays of composites.
Now we do, but the point evidently got overlooked in that patch. Net result
is that updating a field of an array element doesn't work right, as
illustrated if you try the new regression test on an unpatched backend.
Noted while experimenting with EXPLAIN VERBOSE, which has also got some issues
in this area.
Backpatch to 8.3, where arrays of composites were introduced.
Add a note to the documentation of pg_standby that it's important that
the postgres process has permissions to delete the trigger file, per
suggestion by Mason Hale.
Also fix pg_standby to do a more predictable exit(200) instead of the
current exit(-1) when the unlink of the trigger file fails anyway.
This only affects 8.3 branch. Older versions didn't have pg_standby,
and in 8.4 upwards pg_standby is no longer responsible for deleting the
trigger file; it's supposed to be done by recovery_end_command instead.
Joe Conway [Wed, 3 Feb 2010 23:01:34 +0000 (23:01 +0000)]
Check to ensure the number of primary key fields supplied does not
exceed the total number of non-dropped source table fields for
dblink_build_sql_*(). Addresses bug report from Rushabh Lathia.
Tom Lane [Mon, 1 Feb 2010 02:45:41 +0000 (02:45 +0000)]
Change regexp engine's ccondissect/crevdissect routines to perform DFA
matching before recursing instead of after. The DFA match eliminates
unworkable midpoint choices a lot faster than the recursive check, in most
cases, so doing it first can speed things up; particularly in pathological
cases such as recently exhibited by Michael Glaesemann.
In addition, apply some cosmetic changes that were applied upstream (in the
Tcl project) at the same time, in order to sync with upstream version 1.15
of regexec.c.
Upstream apparently intends to backpatch this, so I will too. The
pathological behavior could be unpleasant if encountered in the field,
which seems to justify any risk of introducing new bugs.
Tom Lane, reviewed by Donal K. Fellows of Tcl project
Magnus Hagander [Sun, 31 Jan 2010 17:16:27 +0000 (17:16 +0000)]
Fix race condition in win32 signal handling.
There was a race condition where the receiving pipe could be closed by the
child thread if the main thread was pre-empted before it got a chance to
create a new one, and the dispatch thread ran to completion during that time.
One symptom of this is that rows in pg_listener could be dropped under
heavy load.
Analysis and original patch by Radu Ilie, with some small
modifications by Magnus Hagander.
Tom Lane [Sat, 30 Jan 2010 20:10:05 +0000 (20:10 +0000)]
Avoid performing encoding conversion on command tag strings during EndCommand.
Since all current and foreseeable future command tags will be pure ASCII,
there is no need to do conversion on them. This saves a few cycles and also
avoids polluting otherwise-pristine subtransaction memory contexts, which
is the cause of the backend memory leak exhibited in bug #5302. (Someday
we'll probably want to have a better method of determining whether
subtransaction contexts need to be kept around, but today is not that day.)
Backpatch to 8.0. The cycle-shaving aspect of this would work in 7.4
too, but without subtransactions the memory-leak aspect doesn't apply,
so it doesn't seem worth touching 7.4.
Tom Lane [Mon, 25 Jan 2010 01:58:25 +0000 (01:58 +0000)]
Apply Tcl_Init() to the "hold" interpreter created by pltcl.
You might think this is unnecessary since that interpreter is never used
to run code --- but it turns out that's wrong. As of Tcl 8.5, the "clock"
command (alone among builtin Tcl commands) is partially implemented by
loaded-on-demand Tcl code, which means that it fails if there's not
unknown-command support, and also that it's impossible to run it directly
in a safe interpreter. The way they get around the latter is that
Tcl_CreateSlave() automatically sets up an alias command that forwards any
execution of "clock" in a safe slave interpreter to its parent interpreter.
Thus, when attempting to execute "clock" in trusted pltcl, the command
actually executes in the "hold" interpreter, where it will fail if
unknown-command support hasn't been introduced by sourcing the standard
init.tcl script, which is done by Tcl_Init(). (This is a pretty dubious
design decision on the Tcl boys' part, if you ask me ... but they didn't.)
Back-patch all the way. It's not clear that anyone would try to use ancient
versions of pltcl with a recent Tcl, but it's not clear they wouldn't, either.
Also add a regression test using "clock", in branches that have regression
test support for pltcl.
Tom Lane [Sun, 24 Jan 2010 21:49:39 +0000 (21:49 +0000)]
Fix assorted core dumps and Assert failures that could occur during
AbortTransaction or AbortSubTransaction, when trying to clean up after an
error that prevented (sub)transaction start from completing:
* access to TopTransactionResourceOwner that might not exist
* assert failure in AtEOXact_GUC, if AtStart_GUC not called yet
* assert failure or core dump in AfterTriggerEndSubXact, if
AfterTriggerBeginSubXact not called yet
Per testing by injecting elog(ERROR) at successive steps in StartTransaction
and StartSubTransaction. It's not clear whether all of these cases could
really occur in the field, but at least one of them is easily exposed by
simple stress testing, as per my accidental discovery yesterday.
Tom Lane [Sat, 23 Jan 2010 21:29:12 +0000 (21:29 +0000)]
Insert CHECK_FOR_INTERRUPTS calls into loops in dbsize.c, to ensure that
the various disk-size-reporting functions will respond to query cancel
reasonably promptly even in very large databases. Per report from
Kevin Grittner.
Tom Lane [Mon, 18 Jan 2010 02:30:37 +0000 (02:30 +0000)]
Fix portalmem.c to avoid keeping a dangling pointer to a cached plan list
after it's released its reference count for the cached plan. There are
code paths that might try to examine the plan list before noticing that
the portal is already in aborted state. Report and diagnosis by Tatsuo
Ishii, though this isn't exactly his proposed patch.
Tom Lane [Wed, 13 Jan 2010 23:07:22 +0000 (23:07 +0000)]
When loading critical system indexes into the relcache, ensure we lock the
underlying catalog not only the index itself. Otherwise, if the cache
load process touches the catalog (which will happen for many though not
all of these indexes), we are locking index before parent table, which can
result in a deadlock against processes that are trying to lock them in the
normal order. Per today's failure on buildfarm member gothic_moth; it's
surprising the problem hadn't been identified before.
Back-patch to 8.2. Earlier releases didn't have the issue because they
didn't try to lock these indexes during load (instead assuming that they
couldn't change schema at all during multiuser operation).
Tom Lane [Tue, 12 Jan 2010 18:12:33 +0000 (18:12 +0000)]
Fix relcache reload mechanism to be more robust in the face of errors
occurring during a reload, such as query-cancel. Instead of zeroing out
an existing relcache entry and rebuilding it in place, build a new relcache
entry, then swap its contents with the old one, then free the new entry.
This avoids problems with code believing that a previously obtained pointer
to a cache entry must still reference a valid entry, as seen in recent
failures on buildfarm member jaguar. (jaguar is using CLOBBER_CACHE_ALWAYS
which raises the probability of failure substantially, but the problem
could occur in the field without that.) The previous design was okay
when it was made, but subtransactions and the ResourceOwner mechanism
make it unsafe now.
Also, make more use of the already existing rd_isvalid flag, so that we
remember that the entry requires rebuilding even if the first attempt fails.
Back-patch as far as 8.2. Prior versions have enough issues around relcache
reload anyway (due to inadequate locking) that fixing this one doesn't seem
worthwhile.
Tom Lane [Thu, 7 Jan 2010 19:53:22 +0000 (19:53 +0000)]
Make bit/varbit substring() treat any negative length as meaning "all the rest
of the string". The previous coding treated only -1 that way, and would
produce an invalid result value for other negative values.
We ought to fix it so that 2-parameter bit substring() is a different C
function and the 3-parameter form throws error for negative length, but
that takes a pg_proc change which is impractical in the back branches;
and in any case somebody might be relying on -1 working this way.
So just do this as a back-patchable fix.
Tom Lane [Wed, 30 Dec 2009 03:46:01 +0000 (03:46 +0000)]
Set errno to zero before invoking SSL_read or SSL_write. It appears that
at least in some Windows versions, these functions are capable of returning
a failure indication without setting errno. That puts us into an infinite
loop if the previous value happened to be EINTR. Per report from Brendan
Hill.
Back-patch to 8.2. We could take it further back, but since this is only
known to be an issue on Windows and we don't support Windows before 8.2,
it does not seem worth the trouble.
Previous fix for temporary file management broke returning a set from
PL/pgSQL function within an exception handler. Make sure we use the right
resource owner when we create the tuplestore to hold returned tuples.
Simplify tuplestore API so that the caller doesn't need to be in the right
memory context when calling tuplestore_put* functions. tuplestore.c
automatically switches to the memory context used when the tuplestore was
created. Tuplesort was already modified like this earlier. This patch also
removes the now useless MemoryContextSwitch calls from callers.
Report by Aleksei on pgsql-bugs on Dec 22 2009. Backpatch to 8.1, like
the previous patch that broke this.
Tom Lane [Thu, 24 Dec 2009 17:52:19 +0000 (17:52 +0000)]
Fix wrong WAL info value generated when gistContinueInsert() performs an
index page split. This would result in index corruption, or even more likely
an error during WAL replay, if we were unlucky enough to crash during
end-of-recovery cleanup after having completed an incomplete GIST insertion.
Tom Lane [Sat, 12 Dec 2009 19:24:51 +0000 (19:24 +0000)]
Fix integer-to-bit-string conversions to handle the first fractional byte
correctly when the output bit width is wider than the given integer by
something other than a multiple of 8 bits.
This has been wrong since I first wrote that code for 8.0 :-(. Kudos to
Roman Kononov for being the first to notice, though I didn't use his
patch. Per bug #5237.
Tom Lane [Wed, 9 Dec 2009 21:58:17 +0000 (21:58 +0000)]
Prevent indirect security attacks via changing session-local state within
an allegedly immutable index function. It was previously recognized that
we had to prevent such a function from executing SET/RESET ROLE/SESSION
AUTHORIZATION, or it could trivially obtain the privileges of the session
user. However, since there is in general no privilege checking for changes
of session-local state, it is also possible for such a function to change
settings in a way that might subvert later operations in the same session.
Examples include changing search_path to cause an unexpected function to
be called, or replacing an existing prepared statement with another one
that will execute a function of the attacker's choosing.
The present patch secures VACUUM, ANALYZE, and CREATE INDEX/REINDEX against
these threats, which are the same places previously deemed to need protection
against the SET ROLE issue. GUC changes are still allowed, since there are
many useful cases for that, but we prevent security problems by forcing a
rollback of any GUC change after completing the operation. Other cases are
handled by throwing an error if any change is attempted; these include temp
table creation, closing a cursor, and creating or deleting a prepared
statement. (In 7.4, the infrastructure to roll back GUC changes doesn't
exist, so we settle for rejecting changes of "search_path" in these contexts.)
Original report and patch by Gurjeet Singh, additional analysis by
Tom Lane.
Magnus Hagander [Wed, 9 Dec 2009 06:37:25 +0000 (06:37 +0000)]
Reject certificates with embedded NULLs in the commonName field. This stops
attacks where an attacker would put <attack>\0<propername> in the field and
trick the validation code that the certificate was for <attack>.
This is a very low risk attack since it reuqires the attacker to trick the
CA into issuing a certificate with an incorrect field, and the common
PostgreSQL deployments are with private CAs, and not external ones. Also,
default mode in 8.4 does not do any name validation, and is thus also not
vulnerable - but the higher security modes are.
Backpatch all the way. Even though versions 8.3.x and before didn't have
certificate name validation support, they still exposed this field for
the user to perform the validation in the application code, and there
is no way to detect this problem through that API.
Tom Lane [Wed, 9 Dec 2009 00:36:08 +0000 (00:36 +0000)]
Update time zone data files to tzdata release 2009s: DST law changes in
Antarctica, Argentina, Bangladesh, Fiji, Novokuznetsk, Pakistan, Palestine,
Samoa, Syria. Also historical corrections for Hong Kong.
Fix bug in temporary file management with subtransactions. A cursor opened
in a subtransaction stays open even if the subtransaction is aborted, so
any temporary files related to it must stay alive as well. With the patch,
we use ResourceOwners to track open temporary files and don't automatically
close them at subtransaction end (though in the normal case temporary files
are registered with the subtransaction resource owner and will therefore be
closed).
At end of top transaction, we still check that there's no temporary files
marked as close-at-end-of-transaction open, but that's now just a debugging
cross-check as the resource owner cleanup should've closed them already.
Tom Lane [Wed, 2 Dec 2009 17:41:31 +0000 (17:41 +0000)]
Ignore attempts to set "application_name" in the connection startup packet.
This avoids a useless connection retry and complaint in the postmaster log
when receiving a connection from 8.5 or later libpq.
Backpatch in all supported branches, but of course *not* HEAD.
Tom Lane [Sun, 29 Nov 2009 21:02:28 +0000 (21:02 +0000)]
Fix session-lifespan memory leak when a plperl function is redefined:
we have to tell Perl it can release its compiled copy of the function
text. Noted by Alexey Klyukin.
Back-patch to 8.2 --- the problem exists further back, but this patch
won't work without modification, and it's probably not worth the trouble.
Fix an old bug in multixact and two-phase commit. Prepared transactions can
be part of multixacts, so allocate a slot for each prepared transaction in
the "oldest member" array in multixact.c. On PREPARE TRANSACTION, transfer
the oldest member value from the current backends slot to the prepared xact
slot. Also save and recover the value from the 2pc state file.
The symptom of the bug was that after a transaction prepared, a shared lock
still held by the prepared transaction was sometimes ignored by other
transactions.
Fix back to 8.1, where both 2PC and multixact were introduced.
Tom Lane [Thu, 19 Nov 2009 02:45:50 +0000 (02:45 +0000)]
Fix memory leak in syslogger: logfile_rotate() would leak a copy of the
output filename if CSV logging was enabled and only one of the two possible
output files got rotated during a particular call (which would, in fact,
typically be the case during a size-based rotation). This would amount to
about MAXPGPATH (1KB) per rotation, and it's been there since the CSV
code was put in, so it's surprising that nobody noticed it before.
Per bug #5196 from Thomas Poindessous.
Tom Lane [Tue, 10 Nov 2009 23:12:29 +0000 (23:12 +0000)]
Do not build psql's flex module on its own, but instead include it in
mainloop.c. This ensures that postgres_fe.h is read before including
any system headers, which is necessary to avoid problems on some platforms
where we make nondefault selections of feature macros for stdio.h or
other headers. We have had this policy for flex modules in the backend
for many years, but for some reason it was not applied to psql.
Per trouble report from Alexandra Roy and diagnosis by Albe Laurenz.
Alvaro Herrera [Tue, 10 Nov 2009 18:00:44 +0000 (18:00 +0000)]
Fix longstanding problems in VACUUM caused by untimely interruptions
In VACUUM FULL, an interrupt after the initial transaction has been recorded
as committed can cause postmaster to restart with the following error message:
PANIC: cannot abort transaction NNNN, it was already committed
This problem has been reported many times.
In lazy VACUUM, an interrupt after the table has been truncated by
lazy_truncate_heap causes other backends' relcache to still point to the
removed pages; this can cause future INSERT and UPDATE queries to error out
with the following error message:
could not read block XX of relation 1663/NNN/MMMM: read only 0 of 8192 bytes
The window to this race condition is extremely narrow, but it has been seen in
the wild involving a cancelled autovacuum process.
The solution for both problems is to inhibit interrupts in both operations
until after the respective transactions have been committed. It's not a
complete solution, because the transaction could theoretically be aborted by
some other error, but at least fixes the most common causes of both problems.
Disable triggering failover with a signal in pg_standby on Windows, because
Windows doesn't do signal processing like other platforms do. It never
really worked, but recent changes to the signal handling made it crash.
In PLy_output(), when the elog() call in the TRY branch throws an exception
(this can happen when a statement timeout kicks in, for example), the
PyErr_SetString() call in the CATCH branch can cause a segfault, because the
Py_XDECREF(so) call before it releases memory that is still used by the sv
variable that PyErr_SetString() uses as argument, because sv points into
memory owned by so.
Backpatched back to 8.0, where this code was introduced.
I also threw in a couple of volatile declarations for variables that are used
before and after the TRY. I don't think they caused the crash that I
observed, but they could become issues.
Tom Lane [Sat, 31 Oct 2009 18:12:12 +0000 (18:12 +0000)]
Ensure the previous Perl interpreter selection is restored upon exit from
plperl_call_handler, in both the normal and error-exit paths. Per report
from Alexey Klyukin.
Tom Lane [Fri, 30 Oct 2009 20:58:57 +0000 (20:58 +0000)]
Make the overflow guards in ExecChooseHashTableSize be more protective.
The original coding ensured nbuckets and nbatch didn't exceed INT_MAX,
which while not insane on its own terms did nothing to protect subsequent
code like "palloc(nbatch * sizeof(BufFile *))". Since enormous join size
estimates might well be planner error rather than reality, it seems best
to constrain the initial sizes to be not more than work_mem/sizeof(pointer),
thus ensuring the allocated arrays don't exceed work_mem. We will allow
nbatch to get bigger than that during subsequent ExecHashIncreaseNumBatches
calls, but we should still guard against integer overflow in those palloc
requests. Per bug #5145 from Bernt Marius Johnsen.
Although the given test case only seems to fail back to 8.2, previous
releases have variants of this issue, so patch all supported branches.
Tom Lane [Tue, 27 Oct 2009 20:14:39 +0000 (20:14 +0000)]
Fix AfterTriggerSaveEvent to use a test and elog, not just Assert, to check
that it's called within an AfterTriggerBeginQuery/AfterTriggerEndQuery pair.
The RI cascade triggers suppress that overhead on the assumption that they
are always run non-deferred, so it's possible to violate the condition if
someone mistakenly changes pg_trigger to mark such a trigger deferred.
We don't really care about supporting that, but throwing an error instead
of crashing seems desirable. Per report from Marcelo Costa.
Tom Lane [Fri, 16 Oct 2009 22:08:48 +0000 (22:08 +0000)]
Rewrite pam_passwd_conv_proc to be more robust: avoid assuming that the
pam_message array contains exactly one PAM_PROMPT_ECHO_OFF message.
Instead, deal with however many messages there are, and don't throw error
for PAM_ERROR_MSG and PAM_TEXT_INFO messages. This logic is borrowed from
openssh 5.2p1, which hopefully has seen more real-world PAM usage than we
have. Per bug #5121 from Ryan Douglas, which turned out to be caused by
the conv_proc being called with zero messages. Apparently that is normal
behavior given the combination of Linux pam_krb5 with MS Active Directory
as the domain controller.
Patch all the way back, since this code has been essentially untouched
since 7.4. (Surprising we've not heard complaints before.)
Rename the new MAX_AUTH_TOKEN_LENGTH #define to PG_MAX_AUTH_MAX_TOKEN_LENGTH,
to make it more obvious that it's a PostgreSQL internal limit, not something
that comes from system header files.
Raise the maximum authentication token (Kerberos ticket) size in GSSAPI
and SSPI athentication methods. While the old 2000 byte limit was more than
enough for Unix Kerberos implementations, tickets issued by Windows Domain
Controllers can be much larger.
Tom Lane [Fri, 2 Oct 2009 18:13:19 +0000 (18:13 +0000)]
Fix erroneous handling of shared dependencies (ie dependencies on roles)
in CREATE OR REPLACE FUNCTION. The original code would update pg_shdepend
as if a new function was being created, even if it wasn't, with two bad
consequences: pg_shdepend might record the wrong owner for the function,
and any dependencies for roles mentioned in the function's ACL would be lost.
The fix is very easy: just don't touch pg_shdepend at all when doing a
function replacement.
Also update the CREATE FUNCTION reference page, which never explained
exactly what changes and doesn't change in a function replacement.
In passing, fix the CREATE VIEW reference page similarly; there's no
code bug there, but the docs didn't say what happens.
Tom Lane [Tue, 29 Sep 2009 01:21:02 +0000 (01:21 +0000)]
Fix equivclass.c's not-quite-right strategy for handling X=X clauses.
The original coding correctly noted that these aren't just redundancies
(they're effectively X IS NOT NULL, assuming = is strict). However, they
got treated that way if X happened to be in a single-member EquivalenceClass
already, which could happen if there was an ORDER BY X clause, for instance.
The simplest and most reliable solution seems to be to not try to process
such clauses through the EquivalenceClass machinery; just throw them back
for traditional processing. The amount of work that'd be needed to be
smarter than that seems out of proportion to the benefit.
Per bug #5084 from Bernt Marius Johnsen, and analysis by Andrew Gierth.
Andrew Dunstan [Mon, 28 Sep 2009 17:30:41 +0000 (17:30 +0000)]
Convert a perl array to a postgres array when returned by Set Returning Functions as well as non SRFs. Backpatch to 8.1 where these facilities were introduced. with a little help from Abhijit Menon-Sen.
Tom Lane [Sat, 26 Sep 2009 18:25:03 +0000 (18:25 +0000)]
Fix RelationCacheInitializePhase2 (Phase3, in HEAD) to cope with the
possibility of shared-inval messages causing a relcache flush while it tries
to fill in missing data in preloaded relcache entries. There are actually
two distinct failure modes here:
1. The flush could delete the next-to-be-processed cache entry, causing
the subsequent hash_seq_search calls to go off into the weeds. This is
the problem reported by Michael Brown, and I believe it also accounts
for bug #5074. The simplest fix is to restart the hashtable scan after
we've read any new data from the catalogs. It appears that pre-8.4
branches have not suffered from this failure, because by chance there were
no other catalogs sharing the same hash chains with the catalogs that
RelationCacheInitializePhase2 had work to do for. However that's obviously
pretty fragile, and it seems possible that derivative versions with
additional system catalogs might be vulnerable, so I'm back-patching this
part of the fix anyway.
2. The flush could delete the *current* cache entry, in which case the
pointer to the newly-loaded data would end up being stored into an
already-deleted Relation struct. As long as it was still deleted, the only
consequence would be some leaked space in CacheMemoryContext. But it seems
possible that the Relation struct could already have been recycled, in
which case this represents a hard-to-reproduce clobber of cached data
structures, with unforeseeable consequences. The fix here is to pin the
entry while we work on it.
In passing, also change RelationCacheInitializePhase2 to Assert that
formrdesc() set up the relation's cached TupleDesc (rd_att) with the
correct type OID and hasoids values. This is more appropriate than
silently updating the values, because the original tupdesc might already
have been copied into the catcache. However this part of the patch is
not in HEAD because it fails due to some questionable recent changes in
formrdesc :-(. That will be cleaned up in a subsequent patch.
Don't error out if recycling or removing an old WAL segment fails at the end
of checkpoint. Although the checkpoint has been written to WAL at that point
already, so that all data is safe, and we'll retry removing the WAL segment at
the next checkpoint, if such a failure persists we won't be able to remove any
other old WAL segments either and will eventually run out of disk space. It's
better to treat the failure as non-fatal, and move on to clean any other WAL
segment and continue with any other end-of-checkpoint cleanup.
We don't normally expect any such failures, but on Windows it can happen with
some anti-virus or backup software that lock files without FILE_SHARE_DELETE
flag.
Also, the loop in pgrename() to retry when the file is locked was broken. If a
file is locked on Windows, you get ERROR_SHARE_VIOLATION, not
ERROR_ACCESS_DENIED, at least on modern versions. Fix that, although I left
the check for ERROR_ACCESS_DENIED in there as well (presumably it was correct
in some environment), and added ERROR_LOCK_VIOLATION to be consistent with
similar checks in pgwin32_open(). Reduce the timeout on the loop from 30s to
10s, on the grounds that since it's been broken, we've effectively had a
timeout of 0s and no-one has complained, so a smaller timeout is actually
closer to the old behavior. A longer timeout would mean that if recycling a
WAL file fails because it's locked for some reason, InstallXLogFileSegment()
will hold ControlFileLock for longer, potentially blocking other backends, so
a long timeout isn't totally harmless.
While we're at it, set errno correctly in pgrename().
Backpatch to 8.2, which is the oldest version supported on Windows. The xlog.c
changes would make sense on other platforms and thus on older versions as
well, but since there's no such locking issues on other platforms, it's not
worth it.
On Windows, when a file is deleted and another process still has an open
file handle on it, the file goes into "pending deletion" state where it
still shows up in directory listing, but isn't accessible otherwise. That
confuses RemoveOldXLogFiles(), making it think that the file hasn't been
archived yet, while it actually was, and it was deleted along with the .done
file.
Fix that by renaming the file with ".deleted" extension before deleting it.
Also check the return value of rename() and unlink(), so that if the removal
fails for any reason (e.g another process is holding the file locked), we
don't delete the .done file until the WAL file is really gone.
Backpatch to 8.2, which is the oldest version supported on Windows.
Tom Lane [Tue, 8 Sep 2009 04:25:25 +0000 (04:25 +0000)]
Remove outside-the-scanner references to "yyleng".
It seems the flex developers have decided to change yyleng from int to size_t.
This has already happened in the latest release of OS X, and will start
happening elsewhere once the next release of flex appears. Rather than trying
to divine how it's declared in any particular build, let's just remove the one
existing not-very-necessary external usage.
Back-patch to all supported branches; not so much because users in the field
are likely to care about building old branches with cutting-edge flex, as
to keep OSX-based buildfarm members from having problems with old branches.
Fix encoding handling in xml binary input function. If the XML header didn't
specify an encoding explicitly, we used to treat it as being in database
encoding when we parsed it, but then perform a UTF-8 -> database encoding
conversion on it, which was completely bogus. It's now consistently treated as
UTF-8.
Tom Lane [Thu, 3 Sep 2009 22:11:22 +0000 (22:11 +0000)]
Make LOAD of an already-loaded library into a no-op, instead of attempting
to unload and re-load the library.
The difficulty with unloading a library is that we haven't defined safe
protocols for doing so. In particular, there's no safe mechanism for
getting out of a "hook" function pointer unless libraries are unloaded
in reverse order of loading. And there's no mechanism at all for undefining
a custom GUC variable, so GUC would be left with a pointer to an old value
that might or might not still be valid, and very possibly wouldn't be in
the same place anymore.
While the unload and reload behavior had some usefulness in easing
development of new loadable libraries, it's of no use whatever to normal
users, so just disabling it isn't giving up that much. Someday we might
care to expend the effort to develop safe unload protocols; but even if
we did, there'd be little certainty that every third-party loadable module
was following them, so some security restrictions would still be needed.
Back-patch to 8.2; before that, LOAD was superuser-only anyway.
Security: unprivileged users could crash backend. CVE not assigned yet
Tom Lane [Thu, 3 Sep 2009 22:08:23 +0000 (22:08 +0000)]
Disallow RESET ROLE and RESET SESSION AUTHORIZATION inside security-definer
functions.
This extends the previous patch that forbade SETting these variables inside
security-definer functions. RESET is equally a security hole, since it
would allow regaining privileges of the caller; furthermore it can trigger
Assert failures and perhaps other internal errors, since the code is not
expecting these variables to change in such contexts. The previous patch
did not cover this case because assign hooks don't really have enough
information, so move the responsibility for preventing this into guc.c.
Problem discovered by Heikki Linnakangas.
Security: no CVE assigned yet, extends CVE-2007-6600
Tom Lane [Wed, 2 Sep 2009 02:41:07 +0000 (02:41 +0000)]
Fix pg_ctl's readfile() to not go into infinite loop on an empty file
(could happen if either postgresql.conf or postmaster.opts is empty).
It's been broken since the C version was written for 8.0, so patch
all the way back.
initdb's copy of the function is broken in the same way, but it's
less important there since the input files should never be empty.
Patch that in HEAD only, and also fix some cosmetic differences that
crept into that copy of the function.
Tom Lane [Sun, 30 Aug 2009 16:53:45 +0000 (16:53 +0000)]
Remove duplicate variable initializations identified by clang static checker.
One of these represents a nontrivial bug (a promptly-leaked palloc), so
backpatch.
Alvaro Herrera [Mon, 24 Aug 2009 17:23:28 +0000 (17:23 +0000)]
Avoid calling kill() in a postmaster signal handler.
This causes problems when the system load is high, per report from Zdenek
Kotala in <1250860954.1239.114.camel@localhost>; instead of calling kill
directly, have the signal handler set a flag which is checked in ServerLoop.
This way, the handler can return before being called again by a subsequent
signal sent from the autovacuum launcher. Also, increase the sleep in the
launcher in this failure path to 1 second.
Backpatch to 8.3, which is when the signalling between autovacuum
launcher/postmaster was introduced.
Also, add a couple of ReleasePostmasterChildSlot calls in error paths; this
part backpatched to 8.4 which is when the child slot stuff was introduced.
Tom Lane [Mon, 24 Aug 2009 16:18:25 +0000 (16:18 +0000)]
Fix inclusions of readline/editline header files so that we only attempt to
#include the version of history.h that is in the same directory as the
readline.h we are using. This avoids problems in some scenarios where both
readline and editline are installed. Report and patch by Zdenek Kotala.
Tom Lane [Tue, 18 Aug 2009 21:23:28 +0000 (21:23 +0000)]
Fix overflow for INTERVAL 'x ms' where x is more than a couple million,
and integer datetimes are in use. Per bug report from Hubert Depesz
Lubaczewski.
Tom Lane [Sun, 16 Aug 2009 19:55:38 +0000 (19:55 +0000)]
Fix imprecise documentation of random(): it never returns 1.0.
This was changed in 8.2 but the documentation was not corrected.
Per gripe from Sam Mason.
Tom Lane [Sun, 16 Aug 2009 18:14:46 +0000 (18:14 +0000)]
Fix incorrect encoding-aware name truncation in makeArrayTypeName().
truncate_identifier won't do anything if the passed-in strlen is already
less than NAMEDATALEN, which it always would be given the strlcpy usage.
This has been broken since the arrays-of-composite-types code went in.
Arguably truncate_identifier is suffering from excessive optimization
and should always process the string, but for the moment I'll take the
more localized patch.
Tom Lane [Wed, 12 Aug 2009 18:24:03 +0000 (18:24 +0000)]
Fix old bug in log_autovacuum_min_duration code: it was relying on being able
to access a Relation entry it had just closed. I happened to be testing with
CLOBBER_CACHE_ALWAYS, which made this a guaranteed core dump (at least on
machines where sprintf %s isn't forgiving of a NULL pointer). It's probably
quite unlikely that it would fail in the field, but a bug is a bug. Fix by
moving the relation_close call down past the logging action.
Magnus Hagander [Tue, 11 Aug 2009 11:51:20 +0000 (11:51 +0000)]
Reserve the shared memory region during backend startup on Windows, so
that memory allocated by starting third party DLLs doesn't end up
conflicting with it.
Hopefully this solves the long-time issue with "could not reattach
to shared memory" errors on Win32.
Patch from Tsutomu Yamada and me, based on idea from Trevor Talbot.
Tom Lane [Fri, 7 Aug 2009 20:16:22 +0000 (20:16 +0000)]
Try to defend against the possibility that libpq is still in COPY_IN state
when we reach the post-COPY "pump it dry" error recovery code that was added
2006-11-24. Per a report from Neil Best, there is at least one code path
in which this occurs, leading to an infinite loop in code that's supposed
to be making it more robust not less so. A reasonable response seems to be
to call PQputCopyEnd() again, so let's try that.
Back-patch to all versions that contain the cleanup loop.
Fast shutdown stop should forcibly disconnect any active backends, even
if a smart shutdown is already in progress. Backpatch to 8.3, this was broken
in the patch that introduced "dead-end backends".
Per report by Itagaki Takahiro, patch by Fujii Masao.