os_mbuf_dup() assumes all mbufs are from the same pool

[kenh-sq]

Checklist

• Checked the issue tracker for similar issues to ensure this is not a duplicate
• Read the documentation to confirm the issue is not addressed there and your configuration is set correctly
• Tested with the latest version to ensure the issue hasn't been fixed

How often does this bug occurs?

often

Expected behavior

No Store access faults while handling GATT messages.

Actual behavior (suspected bug)

Original issue posting: espressif/esp-idf#15696

Given a chain of at least 2 mbufs, of which the mbufs come from more than one pool of different-sized buffers and the first mbuf is a smaller size than at least one of the rest, the memcpy() in os_mbuf_dup() will write beyond the limits of the allocated mbuf. This is because os_mbuf_dup() assumes all mbufs in a chain come from the same pool as the first mbuf in the chain.

The fix, I believe, is to move the omp = om->om_omp; line inside the for loop so that the mbuf allocation attempt uses the same pool as the currently iterated mbuf in the original chain.

As noted in the original issue, if the ESP32-C6 ROM implementation is the same as available source code, then this issue definitely exists there as that is the hardware upon which I first experienced this issue.

Error logs or terminal output

Steps to reproduce the behavior

How it was originally discovered:

1. Configure MSYS_1 and MSYS_2 Block Sizes as 2 different values. Ex: 256 and 320 bytes, respectively.
2. Set preferred ATT_MTU to something larger than 252 octets. Ex: 1650
3. Configure project with a GATT characteristic that can accept/handle large writes.
4. Use os_mbuf_dup() to copy the chain from processing in a separate task.
5. Use Android phone to send larger GATT messages (Ex: 517 octets).
6. Repeat step 5 until a Store access fault or other fault occurs.

Likely easier reproduction (not tried, just based on understanding the underlying issue):

1. Configure MSYS_1 and MSYS_2 Block Sizes as 2 different values. Ex: 256 and 320 bytes, respectively.
2. Use os_msys_get() to request a 256-byte-sized mbuf (hopefully getting one from the MSYS_1 pool).
3. Use os_mbuf_append() to fill the mbuf with 256 bytes.
4. Use os_msys_get() to request a 320-byte-size mbuf (hopefully getting one from the MSYS_2 pool).
5. Use os_mbuf_append() to fill the mbuf with 320 bytes.
6. Use os_mbuf_concat() to concatenate the 320-byte mbuf to the tail of the 256-byte mbuf.
7. Attempt os_mbuf_dup() and look for buffer overrun.

Project release version

Commit a9a4c70

System architecture

Intel/AMD 64-bit (modern PC, older Mac)

Operating system

Windows

Operating system version

Windows 11

Shell

Bash

Additional context

No response

[strange969]

Closing the issue since the original issue has been fixed. Feel free to open the issue if you still face any issue.

[kenh-sq]

It's fixed in the C6 compiled library, sure, but the bug still exists in this repo.

Edited to add: I'm not free to reopen the issue as I "do not have permissions to reopen the issue"

[rahult-github]

Hi @kenh-sq ,

Reopening issue. We will check the merge status and update.