Horton

Thread

Commits

  1. Avoid invalid alloc size error in shm_mq

  1. invalid alloc size error possible in shm_mq

    Markus Wanner <markus.wanner@2ndquadrant.com> — 2020-07-07T10:30:35Z

    Hi,

in shm_mq_receive of shm_mq.c, a huge payload may trigger an unjustified 
"invalid memory alloc request size" error due to the way the buffer size 
is increased.

I understand that this fine piece of infrastructure may not have been 
designed for huge payloads.  I still argue it should better not fail 
that way for payloads between half a gigabyte and MaxAllocSize.  The 
attached patch adds an appropriate assertion (documenting the upper 
limit) and avoids the error by limiting the allocation size to 
MaxAllocSize (to correct for such huge payloads below the limit).

We originally hit the error with Postgres 11, but the provided patch 
applies to master and 9.5.  And I suspect to everything in between as well.

The only minor caveat I see from this patch could be that `mqh_buffer` 
would be of size MaxAllocSize, which is not a power of two.  As far as I 
can tell, that does not matter, though.

Please consider applying and back-porting the provided patch.  Thank you.

Best Regards

-- 
Markus Wanner
Senior PostgreSQL Developer
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/

  2. Re: invalid alloc size error possible in shm_mq

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-08-25T10:00:06Z

    On 2020-07-07 12:30, Markus Wanner wrote:
> in shm_mq_receive of shm_mq.c, a huge payload may trigger an unjustified
> "invalid memory alloc request size" error due to the way the buffer size
> is increased.
> 
> I understand that this fine piece of infrastructure may not have been
> designed for huge payloads.  I still argue it should better not fail
> that way for payloads between half a gigabyte and MaxAllocSize.  The
> attached patch adds an appropriate assertion (documenting the upper
> limit) and avoids the error by limiting the allocation size to
> MaxAllocSize (to correct for such huge payloads below the limit).

I wonder if the assertion is appropriate or whether it should be a full 
error check.  Is anything on the sending side ensuring that the maximum 
size is kept?  All the size variables are Size/size_t so could be much 
larger than MaxAllocSize.

Robert, thoughts?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  3. Re: invalid alloc size error possible in shm_mq

    Markus Wanner <markus.wanner@2ndquadrant.com> — 2020-09-09T12:37:17Z

    On 8/25/20 12:00 PM, Peter Eisentraut wrote:
> I wonder if the assertion is appropriate or whether it should be a full 
> error check.

Good point.  Originally, it used to be an error.  With the patch (but 
w/o assertions enabled) it could result in a buffer overrun.  Not good.

I changed the patch to add an error (instead of just an assert) when 
asked to read a message larger than MaxAllocSize.  So this patch 
essentially corrects handling of messages in size between MaxAllocSize/2 
and MaxAllocSize.

> Is anything on the sending side ensuring that the maximum 
> size is kept?  All the size variables are Size/size_t so could be much 
> larger than MaxAllocSize.

In this v2 of the patch, I added a check that errors out on the sender 
side as well (for trying to send a message larger than MaxAllocSize).

I'd still recommend to back-patch this.

-- 
Markus Wanner
Senior PostgreSQL Developer
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/

  4. Re: invalid alloc size error possible in shm_mq

    Robert Haas <robertmhaas@gmail.com> — 2020-10-19T19:31:50Z

    On Wed, Sep 9, 2020 at 8:37 AM Markus Wanner
<markus.wanner@2ndquadrant.com> wrote:
> I'd still recommend to back-patch this.

+1. Sorry for not seeing this thread sooner.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  5. Re: invalid alloc size error possible in shm_mq

    Michael Paquier <michael@paquier.xyz> — 2020-10-20T01:52:25Z

    On Mon, Oct 19, 2020 at 03:31:50PM -0400, Robert Haas wrote:
> +1. Sorry for not seeing this thread sooner.

Please note that this has been applied as of 26ec6b5.
--
Michael

  6. Re: invalid alloc size error possible in shm_mq

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2020-10-20T16:41:38Z

    On 2020-10-20 03:52, Michael Paquier wrote:
> On Mon, Oct 19, 2020 at 03:31:50PM -0400, Robert Haas wrote:
>> +1. Sorry for not seeing this thread sooner.
> 
> Please note that this has been applied as of 26ec6b5.

and backpatched now

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  7. Re: invalid alloc size error possible in shm_mq

    Markus Wanner <markus.wanner@2ndquadrant.com> — 2020-10-20T17:29:02Z

    On 10/20/20 6:41 PM, Peter Eisentraut wrote:
> On 2020-10-20 03:52, Michael Paquier wrote:
>> Please note that this has been applied as of 26ec6b5.
> 
> and backpatched now

Awesome, thanks to everybody who reviewed.

-- 
Markus Wanner
Senior PostgreSQL Developer
2ndQuadrant - PostgreSQL Solutions for the Enterprise
https://www.2ndQuadrant.com/