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

sd-daemon: wipe out memory before using CMSG_NXTHDR() #1540

Merged
merged 1 commit into from Oct 12, 2015

Conversation

zonque
Copy link
Member

@zonque zonque commented Oct 12, 2015

CMSG_NXTHDR() checks for cmsg->cmsg_len after it increased the pointer.
While this makes sense for parsing received messages, that's a pitfall
for code crafting messages with this macro.

Wipe out the allocated memory to fix this.

Fixes #1505
h/t to @dvdhrm :)

CMSG_NXTHDR() checks for cmsg->cmsg_len *after* it increased the pointer.
While this makes sense for parsing received messages, that's a pitfall
for code crafting messages with this macro.

Wipe out the allocated memory to fix this.
@dvdhrm
Copy link
Contributor

dvdhrm commented Oct 12, 2015

Lets see whether "Fixes #1505" is actually true, but looks good!

dvdhrm pushed a commit that referenced this pull request Oct 12, 2015
sd-daemon: wipe out memory before using CMSG_NXTHDR()
@dvdhrm dvdhrm merged commit b7d18f2 into systemd:master Oct 12, 2015
@zonque zonque deleted the cmsg branch October 12, 2015 13:24
@poettering
Copy link
Member

Oh, yuck. This one is nasty! Thanks for tracking this down.

Hmm, I wrote this following the precise description on the cmsg man page, which does not mention this:

       To create ancillary data, first initialize the msg_controllen member of the msghdr with the length of
       the control message buffer.  Use CMSG_FIRSTHDR() on the msghdr to get the first control  message  and
       CMSG_NXTHDR()  to  get  all  subsequent  ones.   In  each  control message, initialize cmsg_len (with
       CMSG_LEN()), the other cmsghdr header fields, and the data portion using CMSG_DATA().   Finally,  the
       msg_controllen  field of the msghdr should be set to the sum of the CMSG_SPACE() of the length of all
       control messages in the buffer. 

I figure we should let the manpages guys know that they should clarify that the memory needs to be NUL initialized...

I wonder if the cmsg_len check is a recent addition, or was always there

@poettering
Copy link
Member

OK, I sent a mail to Michael about this, and put both of you in CC.

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

Successfully merging this pull request may close these issues.

[227 regression] Boot occasionally fails with "Connection timed out"
3 participants