I've been using LLVM's sanitizers and asan turned up a new bit of
compiler behaviour that I hadn't had before. I don't see it in 3.7 or
before, only in their HEAD so I don't know if it's a bug or
intentional.

In numeric.c we have the short numeric headers that have one uint16
(in addition to the varlena header) followed by digits. When compiling
with -O2 on x86-64 LLVM now seems to use a 4-byte access. When there
are no digits (because the number is 0) that overruns the palloced
block.

init_var_from_num: NUMERIC (short) w=0 d=0 POS 6 bytes: 18 00 00 00 00 80
=================================================================
==30523==ERROR: AddressSanitizer: unknown-crash on address
0x6250002c7edc at pc 0x00000144a6d6 bp 0x7ffcfbaadeb0 sp
0x7ffcfbaadea8
READ of size 4 at 0x6250002c7edc thread T0
    #0 0x144a6d5 in init_var_from_num
/home/stark/src/pg/postgresql-master/src/backend/utils/adt/numeric.c:4764:17

Shadow bytes around the buggy address:
=>0x0c4a80050fd0: 00 00 00 00 00 00 f7 f7 00 00 f7[06]00 00 f7 00

   0x000000000144a6d6 <+1078>: mov    $0x2e02680,%edi
   0x000000000144a6db <+1083>: mov    %r14,0x10(%rbx)
   0x000000000144a6df <+1087>: mov    %rsi,%r14

Now, in Postgres currently this is safe for the same reason that the
Form_Data struct accesses are safe. Numerics will always be 4-byte
aligned when allocated in memory. They won't be aligned on disk if
they have a short varlena header but we don't use the GETARG_PP macros
in numeric.c so those always get reallocated in the GETARG macro.

This does represent a hazard though in case we ever do add support for
unaligned packed varlena support to numeric.c. In fact it's extremely
tempting to do so since we never do much work on the varlena form
anyways, we just convert it to NumericVar anyways. It's awfully
annoying that we have to palloc and memcpy to this other intermediate
form just to do it again to the usable internal form. If we did that
we would have to deal with unaligned accesses anyways though so we
would probably have to change this code to just use a char* buffer and
skip the union and struct anyways. Doing so while maintaining binary
on-disk compatibility could be tricky.


-- 
greg


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to