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

Parameter opts is garbage after exiting function ccnl_mkInterest #345

Open
mfrey opened this issue Feb 1, 2019 · 2 comments
Open

Parameter opts is garbage after exiting function ccnl_mkInterest #345

mfrey opened this issue Feb 1, 2019 · 2 comments
Assignees
Labels

Comments

@mfrey
Copy link
Collaborator

mfrey commented Feb 1, 2019

Description

In case a NULL pointer is passed for parameter opts of type ccnl_interest_opts_u in function ccnl_mkInterest a default value is assigned. Unfortunately, the default value is local and thus resides on the stack. Hence, the value of opts is garbage after exiting the function, i.e.

196 int8_t
197 ccnl_mkInterest(struct ccnl_prefix_s *name, ccnl_interest_opts_u *opts,
198                 uint8_t *tmp, uint8_t *tmpend, size_t *len, size_t *offs) {
199     ccnl_interest_opts_u default_opts;
200 
...
221             if (!opts) {
222                 opts = &default_opts;
223             }
...
225             if (!opts->ndntlv.nonce) {
226                 opts->ndntlv.nonce = rand();
227             }

Or am I missing something?

@cgundogan
Copy link
Collaborator

theoretically yes, but once the function ccnl_mkInterest() returns, the opts value is never used again. It's only used for marshalling the packet in ccnl_ndntlv_prependInterest. No references are saved for later use.

@mfrey
Copy link
Collaborator Author

mfrey commented Feb 4, 2019

theoretically yes, but once the function ccnl_mkInterest() returns, the opts value is never used again. It's only used for marshalling the packet in ccnl_ndntlv_prependInterest. No references are saved for later use.

Thanks for the heads up. Honestly, I don't really like it. I've stumbled a couple times in false positives of the static analyzer where e.g. a parameter was used in a subsequent call, etc.

We probably should check how we can silence these false positives or think about if we should clarify/fix these "issues". We also should probably add a new label which allows for easy filtering of these things (check if an issue identified by the static analyzer has been discussed before).

Any thoughts?

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

No branches or pull requests

3 participants