From f4f16af1d38367e68bee456af2d7e284f6aad87d Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Thu, 9 Jul 2020 17:26:49 +0000 Subject: [PATCH] Avoid copying out kernel pointers from msgctl(IPC_STAT). While this behaviour is harmless, it is really just an artifact of the fact that the msgctl(2) implementation uses a user-visible structure as part of the internal implementation, so it is not deliberate and these pointers are not useful to userspace. Thus, NULL them out before copying out, and remove references to them from the manual page. Reported by: Jeffball Reviewed by: emaste, kib MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D25600 --- lib/libc/sys/msgctl.2 | 4 +--- sys/kern/sysv_msg.c | 7 +++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/libc/sys/msgctl.2 b/lib/libc/sys/msgctl.2 index 1ca61ed77b2a..745b365c4d93 100644 --- a/lib/libc/sys/msgctl.2 +++ b/lib/libc/sys/msgctl.2 @@ -31,7 +31,7 @@ .\" .\" $FreeBSD$ .\"/ -.Dd July 9, 2009 +.Dd July 9, 2020 .Dt MSGCTL 2 .Os .Sh NAME @@ -63,8 +63,6 @@ and contains (amongst others) the following members: .Bd -literal struct msqid_ds { struct ipc_perm msg_perm; /* msg queue permission bits */ - struct msg *__msg_first; /* kernel data, don't use */ - struct msg *__msg_last; /* kernel data, don't use */ msglen_t msg_cbytes; /* number of bytes in use on the queue */ msgqnum_t msg_qnum; /* number of msgs in the queue */ msglen_t msg_qbytes; /* max # of bytes on the queue */ diff --git a/sys/kern/sysv_msg.c b/sys/kern/sysv_msg.c index de6faafde286..aab4551357c2 100644 --- a/sys/kern/sysv_msg.c +++ b/sys/kern/sysv_msg.c @@ -613,6 +613,13 @@ kern_msgctl(struct thread *td, int msqid, int cmd, struct msqid_ds *msqbuf) *msqbuf = msqkptr->u; if (td->td_ucred->cr_prison != msqkptr->cred->cr_prison) msqbuf->msg_perm.key = IPC_PRIVATE; + + /* + * Try to hide the fact that the structure layout is shared by + * both the kernel and userland. These pointers are not useful + * to userspace. + */ + msqbuf->__msg_first = msqbuf->__msg_last = NULL; break; default: