Обсуждение: invalid alloc size error possible in shm_mq
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/
Вложения
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
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/
Вложения
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
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
Вложения
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
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/