Hi all, Felix reported weird behaviour on 4.16.0-rc6 with regards to mqueue[1], which was introduced by 36735a6a2b5e ("mqueue: switch to on-demand creation of internal mount"). Basically, the reproducer boils down to being able to mount mqueue if you create a new user namespace, even if you don't unshare the IPC namespace. Previously this was not possible, and you would get an -EPERM. The mount is the *host* mqueue mount, which is being cached and just returned from mqueue_mount(). To be honest, I'm not sure if this is safe or not (or if it was intentional -- since I'm not familiar with mqueue). To me it looks like there is a missing permission check. I've included a patch below that I've compile-tested, and should block the above case. Can someone please tell me if I'm missing something? Is this actually safe? [1]: https://github.com/docker/docker/issues/36674 --8<-------------------------------------------------------------------- Fix a regression caused by 36735a6a2b5e ("mqueue: switch to on-demand creation of internal mount"), where an unprivileged user is permitted to mount mqueue even if they don't have CAP_SYS_ADMIN in the ipcns's associated userns. This can be reproduced as in the following. % unshare -Urm # ipc isn't unshare'd # mount -t mqueue mqueue /dev/mqueue # should have failed # echo $? 0 Previously the above would error out with an -EPERM, as the mount was protected by mount_ns(), but the patch in question switched to kern_mount_data() which doesn't do this necessary permission check. So add it explicitly to mq_internal_mount(). Fixes: 36735a6a2b5e ("mqueue: switch to on-demand creation of internal mount") Reported-by: Felix Abecassis Signed-off-by: Aleksa Sarai --- ipc/mqueue.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ipc/mqueue.c b/ipc/mqueue.c index d7f309f74dec..ddb85091398d 100644 --- a/ipc/mqueue.c +++ b/ipc/mqueue.c @@ -353,6 +353,12 @@ static struct vfsmount *mq_internal_mount(void) { struct ipc_namespace *ns = current->nsproxy->ipc_ns; struct vfsmount *m = ns->mq_mnt; + /* + * Match the semantics of mount_ns, to avoid unprivileged users from being + * able to mount mqueue from an IPC namespace they don't have ownership of. + */ + if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN)) + return ERR_PTR(-EPERM); if (m) return m; m = kern_mount_data(&mqueue_fs_type, ns); -- 2.16.2 -- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH