8000 Fix core dump in ReorderBufferRestoreChange on alignment-picky platfo… · prmdeveloper/postgres@0045691 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0045691

Browse files
tglsfdctglsfdc
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 5daf101 commit 0045691

File tree

3 files changed

+28
-9
lines changed

3 files changed

+28
-9
lines changed

‎contrib/test_decoding/expected/ddl.out

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -183,23 +183,28 @@ CREATE TABLE tr_etoomuch (id serial primary key, data int);
183183
INSERT INTO tr_etoomuch(data) SELECT g.i FROM generate_series(1, 10234) g(i);
184184
DELETE FROM tr_etoomuch WHERE id < 5000;
185185
UPDATE tr_etoomuch SET data = - data WHERE id > 5000;
186+
CREATE TABLE tr_oddlength (id text primary key, data text);
187+
INSERT INTO tr_oddlength VALUES('ab', 'foo');
186188
COMMIT;
187189
/* display results, but hide most of the output */
188190
SELECT count(*), min(data), max(data)
189191
FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1')
190192
GROUP BY substring(data, 1, 24)
191193
ORDER BY 1,2;
192-
count | min | max
193-
-------+-------------------------------------------------+------------------------------------------------------------------------
194-
1 | BEGIN | BEGIN
195-
1 | COMMIT | COMMIT
196-
20467 | table public.tr_etoomuch: DELETE: id[integer]:1 | table public.tr_etoomuch: UPDATE: id[integer]:9999 data[integer]:-9999
197-
(3 rows)
194+
count | min | max
195+
-------+-------------------------------------------------------------------+--------------------------------------------------- 10000 ---------------------
196+
1 | BEGIN | BEGIN
197+
1 | COMMIT | COMMIT
198+
1 | table public.tr_oddlength: INSERT: id[text]:'ab' data[text]:'foo' | table public.tr_oddlength: INSERT: id[text]:'ab' data[text]:'foo'
199+
20467 | table public.tr_etoomuch: DELETE: id[integer]:1 | table public.tr_etoomuch: UPDATE: id[integer]:9999 data[integer]:-9999
200+
(4 rows)
198201

199202
-- check updates of primary keys work correctly
200203
BEGIN;
201204
CREATE TABLE spoolme AS SELECT g.i FROM generate_series(1, 5000) g(i);
202205
UPDATE tr_etoomuch SET id = -id WHERE id = 5000;
206+
UPDATE tr_oddlength SET id = 'x', data = 'quux';
207+
UPDATE tr_oddlength SET id = 'yy', data = 'a';
203208
DELETE FROM spoolme;
204209
DROP TABLE spoolme;
205210
COMMIT;
@@ -209,7 +214,9 @@ WHERE data ~ 'UPDATE';
209214
data
210215
-------------------------------------------------------------------------------------------------------------
211216
table public.tr_etoomuch: UPDATE: old-key: id[integer]:5000 new-tuple: id[integer]:-5000 data[integer]:5000
212-
(1 row)
217+
table public.tr_oddlength: UPDATE: old-key: id[text]:'ab' new-tuple: id[text]:'x' data[text]:'quux'
218+
table public.tr_oddlength: UPDATE: old-key: id[text]:'x' new-tuple: id[text]:'yy' data[text]:'a'
219+
(3 rows)
213220

214221
/*
215222
* check whether we decode subtransactions correctly in relation with each

‎contrib/test_decoding/sql/ddl.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ CREATE TABLE tr_etoomuch (id serial primary key, data int);
106106
INSERT INTO tr_etoomuch(data) SELECT g.i FROM generate_series(1, 10234) g(i);
107107
DELETE FROM tr_etoomuch WHERE id < 5000;
108108
UPDATE tr_etoomuch SET data = - data WHERE id > 5000;
109+
CREATE TABLE tr_oddlength (id text primary key, data text);
110+
INSERT INTO tr_oddlength VALUES('ab', 'foo');
109111
COMMIT;
110112

111113
/* display results, but hide most of the output */
@@ -118,6 +120,8 @@ ORDER BY 1,2;
118120
BEGIN;
119121
CREATE TABLE spoolme AS SELECT g.i FROM generate_series(1, 5000) g(i);
120122
UPDATE tr_etoomuch SET id = -id WHERE id = 5000;
123+
UPDATE tr_oddlength SET id = 'x', data = 'quux';
124+
UPDATE tr_oddlength SET id = 'yy' 8000 ;, data = 'a';
121125
DELETE FROM spoolme;
122126
DROP TABLE spoolme;
123127
COMMIT;

‎src/backend/replication/logical/reorderbuffer.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2276,6 +2276,10 @@ ReorderBufferRestoreChanges(ReorderBuffer *rb, ReorderBufferTXN *txn,
22762276
/*
22772277
* Convert change from its on-disk format to in-memory format and queue it onto
22782278
* the TXN's ->changes list.
2279+
*
2280+
* Note: although "data" is declared char*, at entry it points to a
2281+
* maxalign'd buffer, making it safe in most of this function to assume
2282+
* that the pointed-to data is suitably aligned for direct access.
22792283
*/
22802284
static void
22812285
ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
@@ -2303,7 +2307,7 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
23032307
case REORDER_BUFFER_CHANGE_DELETE:
23042308
if (change->data.tp.oldtuple)
23052309
{
2306-
Size tuplelen = ((HeapTuple) data)->t_len;
2310+
uint32 tuplelen = ((HeapTuple) data)->t_len;
23072311

23082312
change->data.tp.oldtuple =
23092313
ReorderBufferGetTupleBuf(rb, tuplelen - offsetof(HeapTupleHeaderData, t_bits));
@@ -2324,7 +2328,11 @@ ReorderBufferRestoreChange(ReorderBuffer *rb, ReorderBufferTXN *txn,
23242328

23252329
if (change->data.tp.newtuple)
23262330
{
2327-
Size tuplelen = ((HeapTuple) data)->t_len;
2331+
/* here, data might not be suitably aligned! */
2332+
uint32 tuplelen;
2333+
2334+
memcpy(&tuplelen, data + offsetof(HeapTupleData, t_len),
2335+
sizeof(uint32));
23282336

23292337
change->data.tp.newtuple =
23302338
ReorderBufferGetTupleBuf(rb, tuplelen - offsetof(HeapTupleHeaderData, t_bits));

0 commit comments

Comments
 (0)
0