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