1. 29 Jul, 2015 14 commits
  2. 28 Jul, 2015 15 commits
    • Tom Lane's avatar
      Suppress "variable may be used uninitialized" warning. · 2c698f43
      Tom Lane authored
      Also re-pgindent, just because I'm a neatnik.
      2c698f43
    • Joe Conway's avatar
      Disallow converting a table to a view if row security is present. · d824e280
      Joe Conway authored
      When DefineQueryRewrite() is about to convert a table to a view, it checks
      the table for features unavailable to views.  For example, it rejects tables
      having triggers.  It omits to reject tables having relrowsecurity or a
      pg_policy record. Fix that. To faciliate the repair, invent
      relation_has_policies() which indicates the presence of policies on a
      relation even when row security is disabled for that relation.
      
      Reported by Noah Misch. Patch by me, review by Stephen Frost. Back-patch
      to 9.5 where RLS was introduced.
      d824e280
    • Joe Conway's avatar
      Create a pg_shdepend entry for each role in TO clause of policies. · f781a0f1
      Joe Conway authored
      CreatePolicy() and AlterPolicy() omit to create a pg_shdepend entry for
      each role in the TO clause. Fix this by creating a new shared dependency
      type called SHARED_DEPENDENCY_POLICY and assigning it to each role.
      
      Reported by Noah Misch. Patch by me, reviewed by Alvaro Herrera.
      Back-patch to 9.5 where RLS was introduced.
      f781a0f1
    • Tom Lane's avatar
      Update our documentation concerning where to create data directories. · 8c72a7fa
      Tom Lane authored
      Although initdb has long discouraged use of a filesystem mount-point
      directory as a PG data directory, this point was covered nowhere in the
      user-facing documentation.  Also, with the popularity of pg_upgrade,
      we really need to recommend that the PG user own not only the data
      directory but its parent directory too.  (Without a writable parent
      directory, operations such as "mv data data.old" fail immediately.
      pg_upgrade itself doesn't do that, but wrapper scripts for it often do.)
      
      Hence, adjust the "Creating a Database Cluster" section to address
      these points.  I also took the liberty of wordsmithing the discussion
      of NFS a bit.
      
      These considerations aren't by any means new, so back-patch to all
      supported branches.
      8c72a7fa
    • Andrew Dunstan's avatar
      Only adjust negative indexes in json_get up to the length of the path. · 6d10f4e9
      Andrew Dunstan authored
      The previous code resulted in memory access beyond the path bounds. The
      cure is to move it into a code branch that checks the value of lex_level
      is within the correct bounds.
      
      Bug reported and diagnosed by Piotr Stefaniak.
      6d10f4e9
    • Tom Lane's avatar
      Reduce chatter from signaling of autovacuum workers. · d8f15c95
      Tom Lane authored
      Don't print a WARNING if we get ESRCH from a kill() that's attempting
      to cancel an autovacuum worker.  It's possible (and has been seen in the
      buildfarm) that the worker is already gone by the time we are able to
      execute the kill, in which case the failure is harmless.  About the only
      plausible reason for reporting such cases would be to help debug corrupted
      lock table contents, but this is hardly likely to be the most important
      symptom if that happens.  Moreover issuing a WARNING might scare users
      more than is warranted.
      
      Also, since sending a signal to an autovacuum worker is now entirely a
      routine thing, and the worker will log the query cancel on its end anyway,
      reduce the message saying we're doing that from LOG to DEBUG1 level.
      
      Very minor cosmetic cleanup as well.
      
      Since the main practical reason for doing this is to avoid unnecessary
      buildfarm failures, back-patch to all active branches.
      d8f15c95
    • Joe Conway's avatar
      Bump catversion so that HEAD is beyond 9.5 · 1e2bd43b
      Joe Conway authored
      As pointed out by Tom, since HEAD has progressed beyond 9.5 in terms of
      its catalog, we need to be sure catversion of HEAD is advanced beyond
      that of 9.5. Corrects my mistake in the pg_stats view commit cfa928ff.
      1e2bd43b
    • Joe Conway's avatar
      Plug RLS related information leak in pg_stats view. · 7b4bfc87
      Joe Conway authored
      The pg_stats view is supposed to be restricted to only show rows
      about tables the user can read. However, it sometimes can leak
      information which could not otherwise be seen when row level security
      is enabled. Fix that by not showing pg_stats rows to users that would
      be subject to RLS on the table the row is related to. This is done
      by creating/using the newly introduced SQL visible function,
      row_security_active().
      
      Along the way, clean up three call sites of check_enable_rls(). The second
      argument of that function should only be specified as other than
      InvalidOid when we are checking as a different user than the current one,
      as in when querying through a view. These sites were passing GetUserId()
      instead of InvalidOid, which can cause the function to return incorrect
      results if the current user has the BYPASSRLS privilege and row_security
      has been set to OFF.
      
      Additionally fix a bug causing RI Trigger error messages to unintentionally
      leak information when RLS is enabled, and other minor cleanup and
      improvements. Also add WITH (security_barrier) to the definition of pg_stats.
      
      Bumped CATVERSION due to new SQL functions and pg_stats view definition.
      
      Back-patch to 9.5 where RLS was introduced. Reported by Yaroslav.
      Patch by Joe Conway and Dean Rasheed with review and input by
      Michael Paquier and Stephen Frost.
      7b4bfc87
    • Andres Freund's avatar
      Remove ssl renegotiation support. · 426746b9
      Andres Freund authored
      While postgres' use of SSL renegotiation is a good idea in theory, it
      turned out to not work well in practice. The specification and openssl's
      implementation of it have lead to several security issues. Postgres' use
      of renegotiation also had its share of bugs.
      
      Additionally OpenSSL has a bunch of bugs around renegotiation, reported
      and open for years, that regularly lead to connections breaking with
      obscure error messages. We tried increasingly complex workarounds to get
      around these bugs, but we didn't find anything complete.
      
      Since these connection breakages often lead to hard to debug problems,
      e.g. spuriously failing base backups and significant latency spikes when
      synchronous replication is used, we have decided to change the default
      setting for ssl renegotiation to 0 (disabled) in the released
      backbranches and remove it entirely in 9.5 and master.
      
      Author: Andres Freund
      Discussion: 20150624144148.GQ4797@alap3.anarazel.de
      Backpatch: 9.5 and master, 9.0-9.4 get a different patch
      426746b9
    • Andrew Dunstan's avatar
      Make tap tests store postmaster logs and handle vpaths correctly · 01f6bb4b
      Andrew Dunstan authored
      Given this it is possible that the buildfarm animals running these tests
      will be able to capture adequate logging to allow diagnosis of failures.
      01f6bb4b
    • Robert Haas's avatar
      Centralize decision-making about where to get a backend's PGPROC. · 6f2871f1
      Robert Haas authored
      This code was originally written as part of parallel query effort, but
      it seems to have independent value, because if we make one decision
      about where to get a PGPROC when we allocate and then put it back on a
      different list at backend-exit time, bad things happen.  This isn't
      just a theoretical risk; we fixed an actual problem of this type in
      commit e280c630.
      6f2871f1
    • Tom Lane's avatar
      Remove an unsafe Assert, and explain join_clause_is_movable_into() better. · 95f4e59c
      Tom Lane authored
      join_clause_is_movable_into() is approximate, in the sense that it might
      sometimes return "false" when actually it would be valid to push the given
      join clause down to the specified level.  This is okay ... but there was
      an Assert in get_joinrel_parampathinfo() that's only safe if the answers
      are always exact.  Comment out the Assert, and add a bunch of commentary
      to clarify what's going on.
      
      Per fuzz testing by Andreas Seltenreich.  The added regression test is
      a pretty silly query, but it's based on his crasher example.
      
      Back-patch to 9.2 where the faulty logic was introduced.
      95f4e59c
    • Heikki Linnakangas's avatar
      Fix bug in collecting total_latencies from all threads in pgbench. · b2ed8ede
      Heikki Linnakangas authored
      This was broken in 1bc90f7a, which removed the thread-emulation. With modest
      -j and -c settings the result were usually close enough that you wouldn't
      notice it easily, but with a high enough thread count it would access
      uninitialized memory and crash.
      
      Per report from Andres Freund offlist.
      b2ed8ede
    • Heikki Linnakangas's avatar
      Another attempt at fixing memory leak in xlogreader. · 5e65f45c
      Heikki Linnakangas authored
      max_block_id is also reset between reading records.
      
      Michael Paquier
      5e65f45c
    • Joe Conway's avatar
      Fix pg_dump output of policies. · e0d4a290
      Joe Conway authored
      pg_dump neglected to wrap parenthesis around USING and WITH CHECK
      expressions -- fixed. Reported by Noah Misch.
      e0d4a290
  3. 27 Jul, 2015 11 commits
    • Stephen Frost's avatar
      Improve RLS handling in copy.c · 3d5cb31c
      Stephen Frost authored
      To avoid a race condition where the relation being COPY'd could be
      changed into a view or otherwise modified, keep the original lock
      on the relation.  Further, fully qualify the relation when building
      the query up.
      
      Also remove the poorly thought-out Assert() and check the entire
      relationOids list as, post-RLS, there can certainly be multiple
      relations involved and the planner does not guarantee their ordering.
      
      Per discussion with Noah and Andres.
      
      Back-patch to 9.5 where RLS was introduced.
      3d5cb31c
    • Tom Lane's avatar
      Further code review for pg_stat_ssl patch. · 4c8f8ffa
      Tom Lane authored
      Fix additional bogosity in commit 9029f4b3.  Include the
      BackendSslStatusBuffer in the BackendStatusShmemSize calculation,
      avoid ugly and error-prone casts to char* and back, put related
      code stanzas into a consistent order (and fix a couple of previous
      instances of that sin).  All cosmetic except for the size oversight.
      4c8f8ffa
    • Tom Lane's avatar
      Fix pointer-arithmetic thinko in pg_stat_ssl patch. · 7d791ed4
      Tom Lane authored
      Nasty memory-stomp bug in commit 9029f4b3.  It's not apparent how
      this survived even cursory testing :-(.  Per report from Peter Holzer.
      7d791ed4
    • Heikki Linnakangas's avatar
      Don't assume that 'char' is signed. · 5533a272
      Heikki Linnakangas authored
      On some platforms, notably ARM and PowerPC, 'char' is unsigned by
      default. This fixes an assertion failure at WAL replay on such platforms.
      
      Reported by Noah Misch. Backpatch to 9.5, where this was broken.
      5533a272
    • Heikki Linnakangas's avatar
      Fix memory leaks in pg_rewind. Several PQclear() calls were missing. · d7fd22a3
      Heikki Linnakangas authored
      Originally reported by Vladimir Borodin in the pg_rewind github project,
      patch by Michael Paquier.
      d7fd22a3
    • Heikki Linnakangas's avatar
      Don't assume that PageIsEmpty() returns true on an all-zeros page. · 820d1ced
      Heikki Linnakangas authored
      It does currently, and I don't see us changing that any time soon, but we
      don't make that assumption anywhere else.
      
      Per Tom Lane's suggestion. Backpatch to 9.2, like the previous patch that
      added this assumption.
      820d1ced
    • Heikki Linnakangas's avatar
      Fix memory leak in xlogreader facility. · 61a65c53
      Heikki Linnakangas authored
      XLogReaderFree failed to free the per-block data buffers, when they
      happened to not be used by the latest read WAL record.
      
      Michael Paquier. Backpatch to 9.5, where the per-block buffers were added.
      61a65c53
    • Heikki Linnakangas's avatar
      Reuse all-zero pages in GIN. · 33444517
      Heikki Linnakangas authored
      In GIN, an all-zeros page would be leaked forever, and never reused. Just
      add them to the FSM in vacuum, and they will be reinitialized when grabbed
      from the FSM. On master and 9.5, attempting to access the page's opaque
      struct also caused an assertion failure, although that was otherwise
      harmless.
      
      Reported by Jeff Janes. Backpatch to all supported versions.
      33444517
    • Heikki Linnakangas's avatar
      Fix handling of all-zero pages in SP-GiST vacuum. · 023430ab
      Heikki Linnakangas authored
      SP-GiST initialized an all-zeros page at vacuum, but that was not
      WAL-logged, which is not safe. You might get a torn page write, when it gets
      flushed to disk, and end-up with a half-initialized index page. To fix,
      leave it in the all-zeros state, and add it to the FSM. It will be
      initialized when reused. Also don't set the page-deleted flag when recycling
      an empty page. That was also not WAL-logged, and a torn write of that would
      cause the page to have an invalid checksum.
      
      Backpatch to 9.2, where SP-GiST indexes were added.
      023430ab
    • Heikki Linnakangas's avatar
      Avoid calling PageGetSpecialPointer() on an all-zeros page. · 65c384c5
      Heikki Linnakangas authored
      That was otherwise harmless, but tripped the new assertion in
      PageGetSpecialPointer().
      
      Reported by Amit Langote. Backpatch to 9.5, where the assertion was added.
      65c384c5
    • Heikki Linnakangas's avatar
      Remove false comment about speculative insertion. · e3a9a194
      Heikki Linnakangas authored
      There is no full discussion of speculative insertions in the executor
      README. There is a high-level explanation in execIndexing.c, but it doesn't
      seem necessary to refer it from here.
      
      Peter Geoghegan
      e3a9a194