invalid alloc size error possible in shm_mq

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

invalid alloc size error possible in shm_mq

Markus Wanner-3
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/

shm_mq_inv_allocation_fix.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: invalid alloc size error possible in shm_mq

Peter Eisentraut-6
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


Reply | Threaded
Open this post in threaded view
|

Re: invalid alloc size error possible in shm_mq

Markus Wanner-3
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/

shm_mq_inv_allocation_fix_v2.diff (1K) Download Attachment