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

Returning C-array serious flaw #7

Open
ghost opened this issue Jun 19, 2017 · 2 comments
Open

Returning C-array serious flaw #7

ghost opened this issue Jun 19, 2017 · 2 comments

Comments

@ghost
Copy link

ghost commented Jun 19, 2017

The function isotp_continue_receive returns a copy of the structure IsoTpMessage.
But this one also embeds a C-array (payload) that has been allocated within the function, so, on the stack.

Due to this, once the function returns, the C-array points to some freed stack memory, and then may present corrupted data.

I got the issue running ARM based chipset: using the callback is safe, but the returned message contains garbage data.

I 've no idea of how to solve this without changing the API.

@peplin
Copy link
Member

peplin commented Jun 29, 2017

Great catch, thanks! I think an API change is OK, e.g. pass in a pre-allocated IsoTpMessage object by reference to isotp_continue_receive.

@ghost
Copy link
Author

ghost commented Jul 3, 2017

The C standard is not that clear about how complete C-arrays within a struct have to be handled and returned while they were allocated on the stack. The way I understand the C99-standard, it should work. - But I know that some releases of GCC return structs in different ways ( register vs memory) regarding of their size . -

Anyway, IMO, it makes more sense to pass an array allocated on the heap, by the caller.
For two main reasons:

  • be able to allocate the 4096bytes which are required by the isotp standard. Such a bulk of bytes under a stack frame may be an issue for small embedded systems.
  • Do not allocate and free the IsoTpMessage struct each time the caller enters this function. this sounds useless and incurs some cost.

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

1 participant