Re: Re: PROPOSAL: make PostgreSQL sanitizers-friendly (and prevent information disclosure) - Mailing list pgsql-hackers

From Piotr Stefaniak
Subject Re: Re: PROPOSAL: make PostgreSQL sanitizers-friendly (and prevent information disclosure)
Date
Msg-id AM4PR03MB1586797FB8FEC47EF77CFCF1F2160@AM4PR03MB1586.eurprd03.prod.outlook.com
Whole thread Raw
In response to Re: PROPOSAL: make PostgreSQL sanitizers-friendly (and prevent information disclosure)  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: Re: PROPOSAL: make PostgreSQL sanitizers-friendly (and prevent information disclosure)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 2016-08-18 20:02, Heikki Linnakangas wrote:
> On 03/22/2016 03:27 PM, Aleksander Alekseev wrote:
>> diff --git a/src/backend/utils/mmgr/aset.c
>> b/src/backend/utils/mmgr/aset.c
>> index d26991e..46ab8a2 100644
>> --- a/src/backend/utils/mmgr/aset.c
>> +++ b/src/backend/utils/mmgr/aset.c
>> @@ -850,7 +850,7 @@ AllocSetAlloc(MemoryContext context, Size size)
>>              blksize <<= 1;
>>
>>          /* Try to allocate it */
>> -        block = (AllocBlock) malloc(blksize);
>> +        block = (AllocBlock) calloc(1, blksize);
>>
>>          /*
>>           * We could be asking for pretty big blocks here, so cope if
>> malloc

>
> I think this goes too far. You're zeroing all palloc'd memory, even if
> it's going to be passed to palloc0(), and zeroed there. It might even
> silence legitimate warnings, if there's code somewhere that does
> palloc(), and accesses some of it before initializing. Plus it's a
> performance hit.

I just did a test where I
1. memset() that block to 0xAC (aset.c:853)
2. compile and run bin/initdb, then bin/postgres
2. run an SQL file, shut down bin/postgres
3. make a copy of the transaction log file
4. change the memset() to 0x0C, repeat steps 2-3
5. compare the two transaction log files with a combination of
hexdump(1), cut(1), and diff(1).

At the end of the output I can see:

-0f34 0010 0000 f5ff ac02 000a 0000 0000
+0f34 0010 0000 f5ff 0c02 000a 0000 0000

So it looks like the MSan complaint might be a true positive.

The SQL file is just this snippet from bit.sql:
CREATE TABLE varbit_table (a BIT VARYING(16), b BIT VARYING(16));
COPY varbit_table FROM stdin;
X0F    X10
X1F    X11
X2F    X12
X3F    X13
X8F    X04
X000F    X0010
X0123    XFFFF
X2468    X2468
XFA50    X05AF
X1234    XFFF5
\.

I realize given information might be a little bit scarce, but I didn't
know what else might be interesting to you that you wouldn't be able to
reproduce.




pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: standalone backend PANICs during recovery
Next
From: Tom Lane
Date:
Subject: Re: Show dropped users' backends in pg_stat_activity