Skip to content

Conversation

@JohnWestlund
Copy link
Contributor

The current method of moving around the sockaddr data causes compile time buffer overflow warnings:

[ 197s] /usr/include/bits/string3.h:84:3: warning: call to __builtin___memset_chk will always overflow destination buffer [enabled by default]
[ 197s] return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));

by doing the following:
create a temp sockaddr struct pointer that is used in calling parse_uri
memcpy 0's over the struct using sizeofa sockaddr_in or sockaddr_in6 (which is larger and causes the buffer overflow warning)
then upon return
create a mca_oob_tcp_addr_tobject
memcpy the original sockaddr struct over mca_oob_tcp_addr_t->addr (which is a sockaddr_storage struct)

Instead this patch simplifies the process by creating the mca_oob_tcp_addr object first, then passes the pointer to its sockaddr_storage struct so the parse_uri copies in the data directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why leave this line commented out here? If it needs to be deleted, then let's delete it.

@hjelmn
Copy link
Member

hjelmn commented Oct 2, 2015

Is this related to the error coverity is reporting? See CID 710596

@JohnWestlund
Copy link
Contributor Author

@hjelmn That looks like an OPAL defect, this is in ORTE.

Did a quick search for this file in coverity and didn't see any defects listed.

@rhc54
Copy link
Contributor

rhc54 commented Oct 2, 2015

@hjelmn It's a similar problem, but we are only addressing it in the OOB - someone else would have to look at the problem in the TCP BTL.

@rhc54
Copy link
Contributor

rhc54 commented Oct 2, 2015

@JohnWestlund once this clears the automated checks, I think it is good to go.

@JohnWestlund
Copy link
Contributor Author

@rhc54 Great!

rhc54 pushed a commit that referenced this pull request Oct 8, 2015
simplify use of sockaddr* structs to work around buffer overflow warning
@rhc54 rhc54 merged commit 232f97a into open-mpi:master Oct 8, 2015
jsquyres pushed a commit to jsquyres/ompi that referenced this pull request Aug 23, 2016
v2.0.0: fix MPI_TYPE_SET_ATTR with NULL value
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

Successfully merging this pull request may close these issues.

3 participants