New: man page cmsg(3) inconsistency

https://bugzilla.kernel.org/show_bug.cgi?id=15952

URL: http://bugs.gentoo.org/show_bug.cgi?id=318801
Summary: man page cmsg(3) inconsistency
Product: Documentation
Version: unspecified
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: man-pages
AssignedTo: documentation_man-pages-ztI5WcYan/[email protected]
ReportedBy: [email protected]
Regression: No

In man page cmsg(3), I see the following prose:

"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. For more information on the msghdr, see recvmsg(2)."

Then, in the sample code at the bottom, I see this code:

           msg.msg_control = buf;
           msg.msg_controllen = sizeof buf;
           cmsg = CMSG_FIRSTHDR(&msg);
           cmsg->cmsg_level = SOL_SOCKET;
           cmsg->cmsg_type = SCM_RIGHTS;
           cmsg->cmsg_len = CMSG_LEN(sizeof(int) * NUM_FD);
           /* Initialize the payload: */
           fdptr = (int *) CMSG_DATA(cmsg);
           memcpy(fdptr, myfds, NUM_FD * sizeof(int));
           /* Sum of the length of all control messages in the buffer: */
           msg.msg_controllen = cmsg->cmsg_len;

This is inconsistent: the prose states that msg_controllen should be initialized with the sum of CMSG_SPACE across all messages, whereas the example code shows msg_controllen being initialized with cmsg->cmsg_len, which is itself initialized with CMSG_LEN, not CMSG_SPACE.

I don't know what the right answer is, but for control messages whose length is not a multiple of the alignment requirement, this could make a difference.

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

https://bugzilla.kernel.org/show_bug.cgi?id=15952

Josh Bowman <jbowman-2iQDLUlslvR8UrSeD/[email protected]> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |jbowman-2iQDLUlslvR8UrSeD/[email protected]

--- Comment #1 from Josh Bowman <jbowman-2iQDLUlslvR8UrSeD/[email protected]> 2013-06-04 22:36:16 --- I believe the prose section is correct and the sample code is incorrect.

There's a nice chart of the data structure in rfc 2292, section 4.3.

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

https://bugzilla.kernel.org/show_bug.cgi?id=15952

--- Comment #2 from Josh Bowman <jbowman-2iQDLUlslvR8UrSeD/[email protected]> 2013-06-04 22:57:59 --- (Sorry, that's section 4.2 of rfc 2292.)

It looks like there's more detail in Appendix A of rfc 3542 (section 20.2):
"While sending an application may or may not include padding at the end of last ancillary data in msg_controllen and implementations must accept both as valid."

This seems to indicate that the sample code could be correct either way, but only because there's only one control message in the buffer. I think it's still confusing, and should be changed to show msg.msg_controllen initialized with a value coming from CMSG_SPACE. (Or at least to make clear why we can get away with using cmsg_len in this case.)

Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

https://bugzilla.kernel.org/show_bug.cgi?id=15952

Alan <alan-qBU/[email protected]> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |alan-qBU/[email protected]

--- Comment #3 from Alan <alan-qBU/[email protected]> --- Still present

You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

https://bugzilla.kernel.org/show_bug.cgi?id=15952

Michael Kerrisk <[email protected]> changed:

What |Removed |Added
----------------------------------------------------------------------------

             Status|NEW                         |RESOLVED
                 CC|                            |[email protected]
         Resolution|---                         |CODE_FIX

--- Comment #4 from Michael Kerrisk <[email protected]> --- It seems to me that at the time this report was made, there were at least *two* problems with this code snippet:

           msg.msg_control = buf;
[1]        msg.msg_controllen = sizeof buf;
           cmsg = CMSG_FIRSTHDR(&msg);
           cmsg->cmsg_level = SOL_SOCKET;
           cmsg->cmsg_type = SCM_RIGHTS;
           cmsg->cmsg_len = CMSG_LEN(sizeof(int) * NUM_FD);
           /* Initialize the payload: */
           fdptr = (int *) CMSG_DATA(cmsg);
           memcpy(fdptr, myfds, NUM_FD * sizeof(int));
           /* Sum of the length of all control messages in the buffer: */

[2] msg.msg_controllen = cmsg->cmsg_len;

One of these is the problem referred to in this bug, at the line marked [2]. But the other is that there's a general confusion in the code where msg.msg_controllen is being initialized twice.

Since the time of the report, the code has changed a little because on some other reports, but the problem line [2] still exists. The solution is I believe to remove line [2] and modify line [1] (which was already done as a result of the other changes), as shown in this revised code snippet:

           struct msghdr msg = {0};
           struct cmsghdr *cmsg;
           int myfds[NUM_FD]; /* Contains the file descriptors to pass. */
           union {
               /* ancillary data buffer, wrapped in a union in order to ensure
                  it is suitably aligned */
               char buf[CMSG_SPACE(sizeof myfds)];
               struct cmsghdr align;
           } u;
           int *fdptr;

           msg.msg_control = u.buf;
           msg.msg_controllen = sizeof u.buf;
           cmsg = CMSG_FIRSTHDR(&msg);
           cmsg->cmsg_level = SOL_SOCKET;
           cmsg->cmsg_type = SCM_RIGHTS;
           cmsg->cmsg_len = CMSG_LEN(sizeof(int) * NUM_FD);
           /* Initialize the payload: */
           fdptr = (int *) CMSG_DATA(cmsg);
           memcpy(fdptr, myfds, NUM_FD * sizeof(int));

I've made this change, which I believe addresses the problem, so I'm closing this bug. Please reopen, if you believe there is still a problem.

You are receiving this mail because:
You are watching the assignee of the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html