On Thu, Mar 28, 2019 at 08:37:11PM +0100, Tomas Vondra wrote:
>On Thu, Mar 28, 2019 at 07:33:36PM +0100, Tomas Vondra wrote:
>>On Thu, Mar 28, 2019 at 11:29:12AM -0700, Peter Geoghegan wrote:
>>>On Wed, Mar 27, 2019 at 6:27 PM Tomas Vondra
>>><tomas.vondra@2ndquadrant.com> wrote:
>>>>It's a bit too late for pushing emergency fixes over here, so I'll do
>>>>more testing tomorrow and then push.
>>>
>>>The buildfarm is still almost all-red now. Can you estimate how long
>>>it will take to push a fix?
>>>
>>
>>Half an hour, at most. I have a fix and I'm running tests on it to make
>>sure it does break something else.
>>
>
>OK, I've pushed the fix. As explained in the commit message, the
>deserialization was borked in two ways. Firstly, it was vulnerable to
>use-after-free. Secondly, the serialization/deserialization of data for
>by-value types did not work for bigendian systems.
>
>I believe this should fix prion (which was tripping on the first issue,
>due to using -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE) and at
>least some of the bigendian boxes (I've tested it on s390x).
>
OK, this apparently worked fine for all x86 and s390 machines.
>
>I do think there's one remaining issue - the deserialized value is
>allocated as a single chunk, and is then "sliced" into smaller buffers.
>But the code ignores alignment, which I think may trigger SIGBUS on some
>platforms - for example grison, skate or gull fail like this, and those
>are ARMv7 and sparc machines.
>
>I do have a fix for that too, but I decided not to push it yet before
>testing it a bit more.
>
I've pushed a fix for this. The short version is that the serialized
representation was not respecting memory alignment requirements, which was
causing issues in machines sensitive to this (ia64, sparc, hppa). It's a
blind attempt, as I currently don't have access to any such machine.
FWIW I've pushed this now so that the machines can test the other stuff,
like jsonpath. I think the serialization/deserialization would deserve a
second look, and I'm wondering if it should work more like ArrayType (i.e.
relying on att_align_* instead of explicit MAXALIGN).
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services