Skip to content

pool_i.h: Fix to warning for signed/unsigned mismatch. #4389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 9, 2025

Conversation

amubiera
Copy link
Contributor

@amubiera amubiera commented Apr 8, 2025

Fixes new warning added by #4382

@nanangizz nanangizz merged commit 43a652a into pjsip:master Apr 9, 2025
40 of 41 checks passed
@amubiera amubiera deleted the pool-signed-mismatch-warning branch April 9, 2025 23:06
LeonidGoltsblat added a commit to LeonidGoltsblat/pjproject that referenced this pull request Apr 10, 2025
Create a non-expandable pool and request more memory than is left in the current block
LeonidGoltsblat added a commit to LeonidGoltsblat/pjproject that referenced this pull request Apr 11, 2025
The bug in pjsip#4389 (when PJ_POOL_ALIGN_PTR(block->cur, alignment) > block->end) is not yet reproducible consistently on all platforms. The test is temporarily disabled.
@LeonidGoltsblat
Copy link
Contributor

@sauwming, @nanangizz, @amubiera
This recently merged pull request was approved too hastily, introducing a critical issue.
We need to use signed size on the right side of the "if" changed here, but not unsigned on the left side.
The problem occurs when PJ_POOL_ALIGN_PTR(block->cur, alignment) > block->end

As a result, the memory pool may now return memory that falls outside the allocated block boundaries. Does it poses a little risk to the stability of PJSIP? 😄

I tried to reproduce this situation in currently unmerged #4360, but have no stable reproducing on all platforms yet.

@amubiera amubiera restored the pool-signed-mismatch-warning branch April 11, 2025 11:54
@amubiera
Copy link
Contributor Author

@LeonidGoltsblat That is true. PR (#4391) created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants