Skip to content
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

nng_msg_insert: munmap_chunk(): invalid pointer #1808

Closed
RafaelNMeyer opened this issue Apr 14, 2024 · 3 comments
Closed

nng_msg_insert: munmap_chunk(): invalid pointer #1808

RafaelNMeyer opened this issue Apr 14, 2024 · 3 comments

Comments

@RafaelNMeyer
Copy link

Describe the bug
I'm facing a bug when sending a message with a specific number of bytes (if any) from the request to the response socket.

Expected behavior
The message is sent without any errors.

Actual Behavior
I was studying the nng lib to create a request/response socket with IPC protocol. In my testing, I faced a case when sending a specific "string" through the nng_sendmsg function. The explanation and code below will further specify the context:

I allocate a message with nng_msg_alloc, insert the string at the beginning of the message body using nng_msg_insert, and when I call nng_sendmsg the munmap_chunk(): invalid pointer error is thrown.

To get around this, I change nng_msg_insert to nng_msg_append, and no more errors occur.

The funny fact is that with a string lower or higher length than the string specified in the code, the message is sent correctly. I don't know if there is some kind of undefined memory allocation behavior with nng_msg_insert, but it's strange!

Environment Details

  • nng/1.7.3
  • Linux 6.7.9-arch1-1
  • g++ (GCC) 13.2.1 20230801

To Reproduce
client.cpp

#include <nng/nng.h>
#include <nng/protocol/reqrep0/req.h>
#include <string>

auto main() -> int {
	nng_socket s;
	nng_msg* msg;

	s = NNG_SOCKET_INITIALIZER;

	nng_req0_open(&s);
	nng_dial(s, "ipc://nng_test", NULL, 0);
	nng_msg_alloc(&msg, 0);

	std::string body = "'LET THERE BE LIGHT' And there was light.....";
	
  nng_msg_insert(msg, (void *)body.c_str(), body.length());
	nng_sendmsg(s, msg, 0);
	nng_close(s);
}

server.cpp

#include <nng/nng.h>
#include <nng/protocol/reqrep0/rep.h>

auto main() -> int {
	nng_socket s;
	nng_msg* msg;

	s = NNG_SOCKET_INITIALIZER;

	nng_rep0_open(&s);
	nng_listen(s, "ipc://nng_test", NULL, 0);
	nng_msg_alloc(&msg, 0);
	
	nng_recvmsg(s, &msg, 0);
	nng_close(s);

}

To compile:
g++ client.cpp -o client -lnng
g++ server.cpp -o server -lnng

To run:
First run the server and then run the client.

@gdamore
Copy link
Contributor

gdamore commented Apr 24, 2024

My initial attempts at reproduction did not succeed. I'm on a mac though, with m1.

This could differ on 32-bit vs 64-bit cpus as well.

Having said all that, munmap_chunk is not inside NNG itself, and we don't have this munmap_chunk(): invalid pointer string.

This feels like some data corruption somewhere or a problem with your C library perhaps?

If there is a bug in nng here it is most likely in nni_cunk_insert.

I'm going to play with this some more because it should be possible to verify this with just doing trial allocations, inserts, and frees. Stay tuned.

@gdamore
Copy link
Contributor

gdamore commented Apr 24, 2024

Ok, I've confirmed it, and have a fix. Stay tuned.

@gdamore gdamore changed the title Request/reply munmap_chunk(): invalid pointer nng_msg_insert: munmap_chunk(): invalid pointer Apr 24, 2024
gdamore added a commit that referenced this issue Apr 24, 2024
With specific message sizes, we the shuffle of data for msg insert
can calculate the wrong value, leading to heap corruption.
This includes a stress test for msg insert to hopefully exercise
every reasonable edge case.
@gdamore
Copy link
Contributor

gdamore commented Apr 24, 2024

Fixed in master. I'll probably cut a release soon.

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

No branches or pull requests

2 participants