mempool alignment might cause a memory block allocated twice

[wgy0831]

the alignment in mempool is #define _ALIGN4(n) ((((n)+3)/4)*4), it means the alignment might expand the size of the block. If max_sz is not the power of four such as 36, the size of second level is ((((36/4)+3)/4)*4) = 12. Function block_break breaks the big block into 4 small blocks and append the last 3 blocks into the free_list.The last block might overflow the big block because function block_fits can not avoid this overflow. The last small block and the next big block would contain a same memory.

1. max_sz = 36, n_max = 3
2. alloc a small block (size 12)
3. alloc a big block (size 36)
4. alloc 3 small block(size 12)

[wentongwu]

I don't think 36 can be partitioned more, that means in your case the minimum block size should also be 36.

[wgy0831]

I don't think 36 can be partitioned more, that means in your case the minimum block size should also be 36.

Why can't 36 be partitioned more? I change the number to 4100 and the smaller block size is 1028, the case still works because the max_sz is not the power of 4. My point is that the last small block and the next big block would contain a same memory .

[andyross]

Can you turn this into some example code that shows a corrupt block? I don't think this is correct.

It's true that blocks are not required to be a full aligned power-of-four size and that breaking into four can result in trailing blocks that overlap, but the code handles that by adding only the accessible subblocks to the list. That overlapping one you think you are seeing is a phantom; it never gets used.

Unless you have code that actually fails, that is. :) That would be an obvious bug we should fix. But at least as I read this it's an edge case already handled by the design.

[wgy0831]

Can you turn this into some example code that shows a corrupt block? I don't think this is correct.

It's true that blocks are not required to be a full aligned power-of-four size and that breaking into four can result in trailing blocks that overlap, but the code handles that by adding only the accessible subblocks to the list. That overlapping one you think you are seeing is a phantom; it never gets used.

Unless you have code that actually fails, that is. :) That would be an obvious bug we should fix. But at least as I read this it's an edge case already handled by the design.

https://gist.github.com/wgy0831/91bb743d0b56a24d05b330ca7be6a13c

You can easily compile and run this C file. I did what I said before and memset the last small block, then I noticed that the big block has been changed.

[wentongwu]

Can you turn this into some example code that shows a corrupt block? I don't think this is correct.
It's true that blocks are not required to be a full aligned power-of-four size and that breaking into four can result in trailing blocks that overlap, but the code handles that by adding only the accessible subblocks to the list. That overlapping one you think you are seeing is a phantom; it never gets used.
Unless you have code that actually fails, that is. :) That would be an obvious bug we should fix. But at least as I read this it's an edge case already handled by the design.

https://gist.github.com/wgy0831/91bb743d0b56a24d05b330ca7be6a13c

You can easily compile and run this C file. I did what I said before and memset the last small block, then I noticed that the big block has been changed.

I review your code, why don't use SYS_MEM_POOL_DEFINE to do the initialize?

[wgy0831]

Can you turn this into some example code that shows a corrupt block? I don't think this is correct.
It's true that blocks are not required to be a full aligned power-of-four size and that breaking into four can result in trailing blocks that overlap, but the code handles that by adding only the accessible subblocks to the list. That overlapping one you think you are seeing is a phantom; it never gets used.
Unless you have code that actually fails, that is. :) That would be an obvious bug we should fix. But at least as I read this it's an edge case already handled by the design.

https://gist.github.com/wgy0831/91bb743d0b56a24d05b330ca7be6a13c
You can easily compile and run this C file. I did what I said before and memset the last small block, then I noticed that the big block has been changed.

I review your code, why don't use SYS_MEM_POOL_DEFINE to do the initialize?

Because I don't want to involve too many codes such as mutex and other macros that is not about this bug. I think these codes do not change the wrong state. I will appreciate it if you can help me to use SYS_MEM_POOL_DEFINE to do the initialize and show the result. ~~~///(^v^)\~~~

[andyross]

Sorry, didn't look at your sample code until just now. Confirmed, this is a real bug. The root cause is that the "don't add overflowing blocks to the list" step is only done for top level blocks, not for subblocks, so the last block of four may overflow the first block of the next superblock.