Thread: Fix condition in shm_toc and remove unused function shm_toc_freespace.
Hi, hackers
Some conditions in shm_toc_insert and shm_toc_allocate are bogus, like:
if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
Remove the condition `toc_bytes + nbytes < toc_bytes` and take a sizeof(shm_entry) into account in shm_toc_allocate though
shm_toc_allocate does that too.
/* Check for memory exhaustion and overflow. */
- if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
+ if (toc_bytes + sizeof(shm_toc_entry) + nbytes > total_bytes)
{
SpinLockRelease(&toc->toc_mutex);
shm_toc_freespace is introduced with shm_toc by original commit 6ddd5137b2, but is not used since then, so remove it.
Some conditions in shm_toc_insert and shm_toc_allocate are bogus, like:
if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
Remove the condition `toc_bytes + nbytes < toc_bytes` and take a sizeof(shm_entry) into account in shm_toc_allocate though
shm_toc_allocate does that too.
/* Check for memory exhaustion and overflow. */
- if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
+ if (toc_bytes + sizeof(shm_toc_entry) + nbytes > total_bytes)
{
SpinLockRelease(&toc->toc_mutex);
shm_toc_freespace is introduced with shm_toc by original commit 6ddd5137b2, but is not used since then, so remove it.
Regards,
Zhang Mingli
Attachment
Hi,
On Jan 12, 2023, 14:34 +0800, Zhang Mingli <zmlpostgres@gmail.com>, wrote:
On Jan 12, 2023, 14:34 +0800, Zhang Mingli <zmlpostgres@gmail.com>, wrote:
Hi, hackers
Some conditions in shm_toc_insert and shm_toc_allocate are bogus, like:
if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
Remove the condition `toc_bytes + nbytes < toc_bytes` and take a sizeof(shm_entry) into account in shm_toc_allocate though
shm_toc_allocate does that too.
shm_toc_insert does that too, and we can report error earlier.
Regards,
Zhang Mingli
On Thu, Jan 12, 2023 at 2:50 PM Zhang Mingli <zmlpostgres@gmail.com> wrote:
On Jan 12, 2023, 14:34 +0800, Zhang Mingli <zmlpostgres@gmail.com>, wrote:Some conditions in shm_toc_insert and shm_toc_allocate are bogus, like:
if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
Remove the condition `toc_bytes + nbytes < toc_bytes` and take a sizeof(shm_entry) into account in shm_toc_allocate though
shm_toc_allocate does that too.shm_toc_insert does that too, and we can report error earlier.
I don't think we should consider sizeof(shm_toc_entry) in the 'if'
condition in shm_toc_allocate, as this function is not in charge of
allocating a new TOC entry. That's what shm_toc_insert does.
Other parts of this patch look good to me.
Thanks
Richard
condition in shm_toc_allocate, as this function is not in charge of
allocating a new TOC entry. That's what shm_toc_insert does.
Other parts of this patch look good to me.
Thanks
Richard
Hi,
Regards,
Zhang Mingli
On Jan 12, 2023, 16:54 +0800, Richard Guo <guofenglinux@gmail.com>, wrote:
On Thu, Jan 12, 2023 at 2:50 PM Zhang Mingli <zmlpostgres@gmail.com> wrote:On Jan 12, 2023, 14:34 +0800, Zhang Mingli <zmlpostgres@gmail.com>, wrote:Some conditions in shm_toc_insert and shm_toc_allocate are bogus, like:shm_toc_insert does that too, and we can report error earlier.
if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)Remove the condition `toc_bytes + nbytes < toc_bytes` and take a sizeof(shm_entry) into account in shm_toc_allocate though
shm_toc_allocate does that too.
I don't think we should consider sizeof(shm_toc_entry) in the 'if'
condition in shm_toc_allocate, as this function is not in charge of
allocating a new TOC entry. That's what shm_toc_insert does.
Thanks for review.
Make sense.
Even reserve a sizeof(shm_toc_entry) when shm_toc_allocate, it cloud happen that there is no space when shm_toc_insert
in case of other processes may take space after that.
Patch updated.
Make sense.
Even reserve a sizeof(shm_toc_entry) when shm_toc_allocate, it cloud happen that there is no space when shm_toc_insert
in case of other processes may take space after that.
Patch updated.