Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 6a3d396

Browse files
committed
Fix core dump in ReorderBufferRestoreChange on alignment-picky platforms.
When re-reading an update involving both an old tuple and a new tuple from disk, reorderbuffer.c was careless about whether the new tuple is suitably aligned for direct access --- in general, it isn't. We'd missed seeing this in the buildfarm because the contrib/test_decoding tests exercise this code path only a few times, and by chance all of those cases have old tuples with length a multiple of 4, which is usually enough to make the access to the new tuple's t_len safe. For some still-not-entirely-clear reason, however, Debian's sparc build gets a bus error, as reported by Christoph Berg; perhaps it's assuming 8-byte alignment of the pointer? The lack of previous field reports is probably because you need all of these conditions to trigger a crash: an alignment-picky platform (not Intel), a transaction large enough to spill to disk, an update within that xact that changes a primary-key field and has an odd-length old tuple, and of course logical decoding tracing the transaction. Avoid the alignment assumption by using memcpy instead of fetching t_len directly, and add a test case that exposes the crash on picky platforms. Back-patch to 9.4 where the bug was introduced. Discussion: <20160413094117.GC21485@msg.credativ.de>
1 parent c2dc194 commit 6a3d396

File tree

3 files changed

+23
-4
lines changed

3 files changed

+23
-4
lines changed

contrib/test_decoding/expected/ddl.out

+9-2
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,8 @@ SELECT 'tx logical msg' FROM pg_logical_emit_message(true, 'test', 'tx logical m
233233

234234
DELETE FROM tr_etoomuch WHERE id < 5000;
235235
UPDATE tr_etoomuch SET data = - data WHERE id > 5000;
236+
CREATE TABLE tr_oddlength (id text primary key, data text);
237+
INSERT INTO tr_oddlength VALUES('ab', 'foo');
236238
COMMIT;
237239
/* display results, but hide most of the output */
238240
SELECT count(*), min(data), max(data)
@@ -244,13 +246,16 @@ ORDER BY 1,2;
244246
1 | BEGIN | BEGIN
245247
1 | COMMIT | COMMIT
246248
1 | message: transactional: 1 prefix: test, sz: 14 content:tx logical msg | message: transactional: 1 prefix: test, sz: 14 content:tx logical msg
249+
1 | table public.tr_oddlength: INSERT: id[text]:'ab' data[text]:'foo' | table public.tr_oddlength: INSERT: id[text]:'ab' data[text]:'foo'
247250
20467 | table public.tr_etoomuch: DELETE: id[integer]:1 | table public.tr_etoomuch: UPDATE: id[integer]:9999 data[integer]:-9999
248-
(4 rows)
251+
(5 rows)
249252

250253
-- check updates of primary keys work correctly
251254
BEGIN;
252255
CREATE TABLE spoolme AS SELECT g.i FROM generate_series(1, 5000) g(i);
253256
UPDATE tr_etoomuch SET id = -id WHERE id = 5000;
257+
UPDATE tr_oddlength SET id = 'x', data = 'quux';
258+
UPDATE tr_oddlength SET id = 'yy', data = 'a';
254259
DELETE FROM spoolme;
255260
DROP TABLE spoolme;
256261
COMMIT;
@@ -260,7 +265,9 @@ WHERE data ~ 'UPDATE';
260265
data
261266
-------------------------------------------------------------------------------------------------------------
262267
table public.tr_etoomuch: UPDATE: old-key: id[integer]:5000 new-tuple: id[integer]:-5000 data[integer]:5000
263-
(1 row)
268+
table public.tr_oddlength: UPDATE: old-key: id[text]:'ab' new-tuple: id[text]:'x' data[text]:'quux'
269+
table public.tr_oddlength: UPDATE: old-key: id[text]:'x' new-tuple: id[text]:'yy' data[text]:'a'
270+
(3 rows)
264271

265272
-- check that a large, spooled, upsert works
266273
INSERT INTO tr_etoomuch (id, data)

contrib/test_decoding/sql/ddl.sql

+4
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ INSERT INTO tr_etoomuch(data) SELECT g.i FROM generate_series(1, 10234) g(i);
116116
SELECT 'tx logical msg' FROM pg_logical_emit_message(true, 'test', 'tx logical msg');
117117
DELETE FROM tr_etoomuch WHERE id < 5000;
118118
UPDATE tr_etoomuch SET data = - data WHERE id > 5000;
119+
CREATE TABLE tr_oddlength (id text primary key, data text);
120+
INSERT INTO tr_oddlength VALUES('ab', 'foo');
119121
COMMIT;
120122

121123
/* display results, but hide most of the output */
@@ -128,6 +130,8 @@ ORDER BY 1,2;
128130
BEGIN;
129131
CREATE TABLE spoolme AS SELECT g.i FROM generate_series(1, 5000) g(i);
130132
UPDATE tr_etoomuch SET id = -id WHERE id = 5000;
133+
UPDATE tr_oddlength SET id = 'x', data = 'quux';
134+
UPDATE tr_oddlength SET id = 'yy', data = 'a';
131135
DELETE FROM spoolme;
132136
DROP TABLE spoolme;
133137
COMMIT;

src/backend/replication/logical/reorderbuffer.c

+10-2
Original file line numberDiff line numberDiff line change
@@ -2444,6 +2444,10 @@ ReorderBufferRestoreChanges(ReorderBuffer *rb, ReorderBufferTXN *txn,
24442444
/*
24452445
* Convert change from its on-disk format to in-memory format and queue it onto
24462446
* the TXN's ->changes list.
2447+
*
2448+
* Note: although "data" is declared char*, at entry it points to a
2449+
* maxalign'd buffer, making it safe in most of this function to assume
2450+
* that the pointed-to data is suitably aligned for direct access.
24472451
*/
24482452
static void
24492453
ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
@@ -2471,7 +2475,7 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
24712475
case REORDER_BUFFER_CHANGE_INTERNAL_SPEC_INSERT:
24722476
if (change->data.tp.oldtuple)
24732477
{
2474-
Size tuplelen = ((HeapTuple) data)->t_len;
2478+
uint32 tuplelen = ((HeapTuple) data)->t_len;
24752479

24762480
change->data.tp.oldtuple =
24772481
ReorderBufferGetTupleBuf(rb, tuplelen - SizeofHeapTupleHeader);
@@ -2492,7 +2496,11 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
24922496

24932497
if (change->data.tp.newtuple)
24942498
{
2495-
Size tuplelen = ((HeapTuple) data)->t_len;
2499+
/* here, data might not be suitably aligned! */
2500+
uint32 tuplelen;
2501+
2502+
memcpy(&tuplelen, data + offsetof(HeapTupleData, t_len),
2503+
sizeof(uint32));
24962504

24972505
change->data.tp.newtuple =
24982506
ReorderBufferGetTupleBuf(rb, tuplelen - SizeofHeapTupleHeader);

0 commit comments

Comments
 (0)