NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
lib/59054: cmsg(3) uses malloc(3) unnecessarily
>Number: 59054
>Category: lib
>Synopsis: cmsg(3) uses malloc(3) unnecessarily
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: lib-bug-people
>State: open
>Class: doc-bug
>Submitter-Id: net
>Arrival-Date: Fri Feb 07 16:10:00 +0000 2025
>Originator: Taylor R Campbell
>Release: current, 10, 9, ...
>Organization:
The NetCMSG Alignment
>Environment:
>Description:
For a handful of NetBSD releases over a decade ago, the CMSG_SPACE macro expanded to a non-constant expression, as a hokey workaround for some ABI issue I have long since forgotten about. That was noncompliant with POSIX, which requires:
`If the argument is an integer constant expression, this macro shall expand to an integer constant expression.'
https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/sys_socket.h.html
Since 2012, we have fixed CMSG_SPACE (and CMSG_LEN) so it is a constant expression (and if the ABI issue is still an issue, well, we can use compat_* to sort it out, if it hasn't already been sorted out):
https://mail-index.netbsd.org/source-changes/2012/01/20/msg030814.html
But while CMSG_SPACE was broken, the man page was updated to dynamically allocate struct cmsghdr objects to avoid variable-length arrays in the example code. This use of malloc/free is unnecessary, now that CMSG_SPACE is fixed -- we should just allocate statically sized buffers on the stack with it.
>How-To-Repeat:
man cmsg
>Fix:
(filing a PR even though I've already drafted this patch, in order to track pullups)
Index: share/man/man3/CMSG_DATA.3
===================================================================
RCS file: /cvsroot/src/share/man/man3/CMSG_DATA.3,v
retrieving revision 1.3
diff -p -p -u -r1.3 CMSG_DATA.3
--- share/man/man3/CMSG_DATA.3 24 Jan 2015 17:17:01 -0000 1.3
+++ share/man/man3/CMSG_DATA.3 7 Feb 2025 16:06:47 -0000
@@ -98,19 +98,13 @@ struct msghdr msg;
struct cmsghdr *cmsg;
/* We use a union to make sure hdr is aligned */
union {
- struct cmsghdr hdr;
- unsigned char buf[CMSG_SPACE(sizeof(int))];
-} *cmsgbuf;
+ struct cmsghdr hdr;
+ unsigned char buf[CMSG_SPACE(sizeof(int))];
+} cmsgbuf;
-/*
- * We allocate in the heap instead of the stack to avoid C99
- * variable stack allocation, which breaks gcc -fstack-protector.
- */
-if ((cmsgbuf = malloc(sizeof(*cmsgbuf))) == NULL)
- err(1, "malloc");
(void)memset(&msg, 0, sizeof(msg));
-msg.msg_control = cmsgbuf->buf;
-msg.msg_controllen = sizeof(cmsgbuf->buf);
+msg.msg_control = cmsgbuf.buf;
+msg.msg_controllen = sizeof(cmsgbuf.buf);
cmsg = CMSG_FIRSTHDR(&msg);
cmsg->cmsg_len = CMSG_LEN(sizeof(int));
@@ -120,7 +114,6 @@ cmsg->cmsg_type = SCM_RIGHTS;
if (sendmsg(s, &msg, 0) == -1)
err(1, "sendmsg");
-free(cmsgbuf);
.Ed
.Pp
And an example that receives and decomposes the control message:
@@ -128,15 +121,13 @@ And an example that receives and decompo
struct msghdr msg;
struct cmsghdr *cmsg;
union {
- struct cmsghdr hdr;
- unsigned char buf[CMSG_SPACE(sizeof(int))];
-} *cmsgbuf;
+ struct cmsghdr hdr;
+ unsigned char buf[CMSG_SPACE(sizeof(int))];
+} cmsgbuf;
-if ((cmsgbuf = malloc(sizeof(*cmsgbuf))) == NULL)
- err(1, "malloc");
(void)memset(&msg, 0, sizeof(msg));
-msg.msg_control = cmsgbuf->buf;
-msg.msg_controllen = sizeof(cmsgbuf->buf);
+msg.msg_control = cmsgbuf.buf;
+msg.msg_controllen = sizeof(cmsgbuf.buf);
if (recvmsg(s, &msg, 0) == -1)
err(1, "recvmsg");
@@ -151,7 +142,6 @@ for (cmsg = CMSG_FIRSTHDR(&msg); cmsg !=
/* Do something with the descriptor. */
}
}
-free(cmsgbuf);
.Ed
.Sh SEE ALSO
.Xr recvmsg 2 ,
Home |
Main Index |
Thread Index |
Old Index