From nobody Sun Nov 03 16:48:08 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4XhLBd1Vrjz5cRHg; Sun, 03 Nov 2024 16:48:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4XhLBd0kjLz4XRN; Sun, 3 Nov 2024 16:48:09 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1730652489; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2jA9fWMWEXtV4jh5Qgt1ZF/aL5ZpZcPm6VPvsDwXarY=; b=HZl/6oYGwsO0ZfGTeIBu/PUaHBS77Vsfepsd1z2yLWLr2pcsHtyvJ2JErGLOvAaNqo5lA/ lIDfD4CAnxb/SjUyQfdYBuXspoHXTAH3xM3A6nBp8+aNnAn31Rs9017Jc0h8NHRXnjK0MJ QOxW84VQwvmJVeX+MVtide2sqyNl3XlRSeK1cNKyCtrWMNCMGcEWWVTK2J15VCVwtfNgyQ XiECTAFdGcv3NrJ0AgbI2yrYapD3gftoPIdwnZpZ7ejxlGCrud2KN+VQPu9CnSfhhv1qEh ysXarz+VcB8S6+qLMPkotaoHPO8Fi71R+Pqe24Kj0qW1b37bPGej/vINYO7hYw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1730652489; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2jA9fWMWEXtV4jh5Qgt1ZF/aL5ZpZcPm6VPvsDwXarY=; b=P1kwOr53dF74lwxAO4ZnVfnj4SnB5pYoKHhJl/ckKHArX/jUQJai12qXdCSlrEXbMh2opW uJqvdV53wvTjxUcHgRWvobhR6Q3IgJJ0T4EOuHRC7Phhk+0zhc40elwS5XTtdI10GzF61n 6PMGmgA8jkUkW2OfVhE6RLmR0WI8ruOHORCnIc96uUKJNTLAxjku6lRSomNWwksr329KAw VFAmOFx2hhk63M7DYzDAaBrjPIQn2QZI6UzvVUhazKAcs145aE/pW2Do+v4ATMMAv5QcBy Gol7qGmxhiDEn6YvVIjEwDTzHI84n5nxGOgFraGMB1WyGVKoa1w4i3aTeuJpeg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1730652489; a=rsa-sha256; cv=none; b=kHqKHW7tBeU8FyK+/2XkuExj9QRvAyEUCDzR1IKTbeWdVdU8VKueOKNjklKNPePGOO+9qY K7hVjtPA6+EAkq782kP/9p6Ohh7wRYqAjPeLdIrB9QmSu6pHFn1RuL5HVosjtaFNglz63n U1rd8Bb+CCo1rx0kC5uF2XPjS4/UZMnPed7kqyioav/co1RhI0Mx5PWu/MLvWd5qOlaJnr cWiN3ub7BxjibdTu5yI6tFs0zjyJjvlZW2pcht7pKL9s+Qupm4eiI9PFTzZVZiVfDwNtcc arzhbfk+XkxmO6SkLIh3rbfwxoG2EOx9OPAFXtPguJ/7GTvpnzowD4Fbg0wQNQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4XhLBd0K7xzwh0; Sun, 3 Nov 2024 16:48:09 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 4A3Gm8t4092249; Sun, 3 Nov 2024 16:48:08 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 4A3Gm8Ap092246; Sun, 3 Nov 2024 16:48:08 GMT (envelope-from git) Date: Sun, 3 Nov 2024 16:48:08 GMT Message-Id: <202411031648.4A3Gm8Ap092246@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: bfd03046d187 - main - unix: Add support for atomically setting the socket mode List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: bfd03046d18776ea70785ca1ef36dfc60822de3b Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=bfd03046d18776ea70785ca1ef36dfc60822de3b commit bfd03046d18776ea70785ca1ef36dfc60822de3b Author: Mark Johnston AuthorDate: 2024-11-03 14:39:32 +0000 Commit: Mark Johnston CommitDate: 2024-11-03 16:46:53 +0000 unix: Add support for atomically setting the socket mode With this patch, it is possible to call fchmod() on a unix socket prior to binding it to the filesystem namespace, so that the mode is set atomically. Without this, one has to call chmod() after bind(), leaving a window where threads can connect to the socket with the default mode. After bind(), fchmod() reverts to failing with EINVAL. This interface is copied from Linux. The behaviour of fstat() is unmodified, i.e., it continues to return the mode as set by soo_stat(). PR: 282393 Reviewed by: kib MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D47361 --- lib/libsys/chmod.2 | 9 ++++++++- share/man/man4/unix.4 | 12 +++++++++++- sys/kern/sys_socket.c | 17 ++++++++++++++++- sys/kern/uipc_usrreq.c | 29 ++++++++++++++++++++++++++++- sys/sys/protosw.h | 5 +++++ sys/sys/unpcb.h | 1 + tests/sys/kern/unix_dgram.c | 41 ++++++++++++++++++++++++++++++++++++++++- 7 files changed, 109 insertions(+), 5 deletions(-) diff --git a/lib/libsys/chmod.2 b/lib/libsys/chmod.2 index e496b773ddf8..2db182a95fcc 100644 --- a/lib/libsys/chmod.2 +++ b/lib/libsys/chmod.2 @@ -25,7 +25,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd March 30, 2021 +.Dd October 31, 2024 .Dt CHMOD 2 .Os .Sh NAME @@ -214,6 +214,13 @@ This makes the system somewhat more secure by protecting set-user-id (set-group-id) files from remaining set-user-id (set-group-id) if they are modified, at the expense of a degree of compatibility. +.Pp +While it is normally an error to invoke +.Fn fchmod +on a socket, it is possible to do so on +.Dv AF_LOCAL +sockets before they are bound to a file name; see +.Xr unix 4 . .Sh RETURN VALUES .Rv -std .Sh ERRORS diff --git a/share/man/man4/unix.4 b/share/man/man4/unix.4 index 5ac9ccd5514f..2fdfde225b14 100644 --- a/share/man/man4/unix.4 +++ b/share/man/man4/unix.4 @@ -25,7 +25,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd February 1, 2022 +.Dd October 31, 2024 .Dt UNIX 4 .Os .Sh NAME @@ -77,6 +77,15 @@ removed when the socket is closed \(em .Xr unlink 2 must be used to remove the file. .Pp +Prior to binding a socket, +.Xr fchmod 2 +can be used to set the permissions of the socket file. +This avoids the race that would otherwise occur between creation of the file +and a subsequent call to +.Xr chmod 2 . +Once the socket is bound to a file name, the permissions of the file can not be +changed this way. +.Pp The length of .Ux Ns -domain address, required by @@ -441,6 +450,7 @@ The order is preserved for writes coming through a particular connection. .Sh SEE ALSO .Xr connect 2 , .Xr dup 2 , +.Xr fchmod 2 , .Xr fcntl 2 , .Xr getsockopt 2 , .Xr listen 2 , diff --git a/sys/kern/sys_socket.c b/sys/kern/sys_socket.c index 55837e4cb6ce..65d864e4f9f4 100644 --- a/sys/kern/sys_socket.c +++ b/sys/kern/sys_socket.c @@ -90,6 +90,7 @@ static fo_poll_t soo_poll; extern fo_kqfilter_t soo_kqfilter; static fo_stat_t soo_stat; static fo_close_t soo_close; +static fo_chmod_t soo_chmod; static fo_fill_kinfo_t soo_fill_kinfo; static fo_aio_queue_t soo_aio_queue; @@ -104,7 +105,7 @@ struct fileops socketops = { .fo_kqfilter = soo_kqfilter, .fo_stat = soo_stat, .fo_close = soo_close, - .fo_chmod = invfo_chmod, + .fo_chmod = soo_chmod, .fo_chown = invfo_chown, .fo_sendfile = invfo_sendfile, .fo_fill_kinfo = soo_fill_kinfo, @@ -353,6 +354,20 @@ soo_close(struct file *fp, struct thread *td) return (error); } +static int +soo_chmod(struct file *fp, mode_t mode, struct ucred *cred, struct thread *td) +{ + struct socket *so; + int error; + + so = fp->f_data; + if (so->so_proto->pr_chmod != NULL) + error = so->so_proto->pr_chmod(so, mode, cred, td); + else + error = EINVAL; + return (error); +} + static int soo_fill_kinfo(struct file *fp, struct kinfo_file *kif, struct filedesc *fdp) { diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c index fc1559fb1fb1..7af73a1d344b 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -484,6 +484,7 @@ uipc_attach(struct socket *so, int proto, struct thread *td) unp->unp_socket = so; so->so_pcb = unp; refcount_init(&unp->unp_refcount, 1); + unp->unp_mode = ACCESSPERMS; if ((locked = UNP_LINK_WOWNED()) == false) UNP_LINK_WLOCK(); @@ -526,6 +527,7 @@ uipc_bindat(int fd, struct socket *so, struct sockaddr *nam, struct thread *td) struct mount *mp; cap_rights_t rights; char *buf; + mode_t mode; if (nam->sa_family != AF_UNIX) return (EAFNOSUPPORT); @@ -558,6 +560,7 @@ uipc_bindat(int fd, struct socket *so, struct sockaddr *nam, struct thread *td) return (EALREADY); } unp->unp_flags |= UNP_BINDING; + mode = unp->unp_mode & ~td->td_proc->p_pd->pd_cmask; UNP_PCB_UNLOCK(unp); buf = malloc(namelen + 1, M_TEMP, M_WAITOK); @@ -590,7 +593,7 @@ restart: } VATTR_NULL(&vattr); vattr.va_type = VSOCK; - vattr.va_mode = (ACCESSPERMS & ~td->td_proc->p_pd->pd_cmask); + vattr.va_mode = mode; #ifdef MAC error = mac_vnode_check_create(td->td_ucred, nd.ni_dvp, &nd.ni_cnd, &vattr); @@ -702,6 +705,27 @@ uipc_close(struct socket *so) } } +static int +uipc_chmod(struct socket *so, mode_t mode, struct ucred *cred __unused, + struct thread *td __unused) +{ + struct unpcb *unp; + int error; + + if ((mode & ~ACCESSPERMS) != 0) + return (EINVAL); + + error = 0; + unp = sotounpcb(so); + UNP_PCB_LOCK(unp); + if (unp->unp_vnode != NULL || (unp->unp_flags & UNP_BINDING) != 0) + error = EINVAL; + else + unp->unp_mode = mode; + UNP_PCB_UNLOCK(unp); + return (error); +} + static int uipc_connect2(struct socket *so1, struct socket *so2) { @@ -3357,6 +3381,7 @@ static struct protosw streamproto = { .pr_sockaddr = uipc_sockaddr, .pr_soreceive = soreceive_generic, .pr_close = uipc_close, + .pr_chmod = uipc_chmod, }; static struct protosw dgramproto = { @@ -3380,6 +3405,7 @@ static struct protosw dgramproto = { .pr_sockaddr = uipc_sockaddr, .pr_soreceive = uipc_soreceive_dgram, .pr_close = uipc_close, + .pr_chmod = uipc_chmod, }; static struct protosw seqpacketproto = { @@ -3411,6 +3437,7 @@ static struct protosw seqpacketproto = { .pr_sockaddr = uipc_sockaddr, .pr_soreceive = soreceive_generic, /* XXX: or...? */ .pr_close = uipc_close, + .pr_chmod = uipc_chmod, }; static struct domain localdomain = { diff --git a/sys/sys/protosw.h b/sys/sys/protosw.h index 1d691166929f..f9d7e1c4bb69 100644 --- a/sys/sys/protosw.h +++ b/sys/sys/protosw.h @@ -32,6 +32,8 @@ #ifndef _SYS_PROTOSW_H_ #define _SYS_PROTOSW_H_ +#include + /* Forward declare these structures referenced from prototypes below. */ struct kaiocb; struct mbuf; @@ -100,6 +102,8 @@ typedef int pr_bindat_t(int, struct socket *, struct sockaddr *, typedef int pr_connectat_t(int, struct socket *, struct sockaddr *, struct thread *); typedef int pr_aio_queue_t(struct socket *, struct kaiocb *); +typedef int pr_chmod_t(struct socket *, __mode_t, struct ucred *, + struct thread *); struct protosw { short pr_type; /* socket type used for */ @@ -139,6 +143,7 @@ struct protosw { pr_sense_t *pr_sense; /* stat(2) */ pr_sosetlabel_t *pr_sosetlabel; /* MAC, XXXGL: remove */ pr_setsbopt_t *pr_setsbopt; /* Socket buffer ioctls */ + pr_chmod_t *pr_chmod; /* fchmod(2) */ }; /*#endif*/ diff --git a/sys/sys/unpcb.h b/sys/sys/unpcb.h index d22662fe83e5..87611fd9aee4 100644 --- a/sys/sys/unpcb.h +++ b/sys/sys/unpcb.h @@ -93,6 +93,7 @@ struct unpcb { u_int unp_msgcount; /* (g) references from message queue */ u_int unp_gcrefs; /* (g) garbage collector refcount */ ino_t unp_ino; /* (g) fake inode number */ + mode_t unp_mode; /* (g) initial pre-bind() mode */ LIST_ENTRY(unpcb) unp_dead; /* (g) link in dead list */ } __aligned(CACHE_LINE_SIZE); diff --git a/tests/sys/kern/unix_dgram.c b/tests/sys/kern/unix_dgram.c index 9df0d4ca7168..7464cdf197cd 100644 --- a/tests/sys/kern/unix_dgram.c +++ b/tests/sys/kern/unix_dgram.c @@ -25,13 +25,15 @@ * SUCH DAMAGE. */ -#include #include #include #include #include +#include #include +#include #include + #include #include #include @@ -391,12 +393,49 @@ ATF_TC_BODY(selfgetpeername, tc) ATF_REQUIRE(close(sd) == 0); } +ATF_TC_WITHOUT_HEAD(fchmod); +ATF_TC_BODY(fchmod, tc) +{ + struct stat sb; + struct sockaddr_un sun; + int error, sd; + + memset(&sun, 0, sizeof(sun)); + sun.sun_len = sizeof(sun); + sun.sun_family = AF_UNIX; + strlcpy(sun.sun_path, "sock", sizeof(sun.sun_path)); + + sd = socket(PF_UNIX, SOCK_DGRAM, 0); + ATF_REQUIRE(sd != -1); + + error = fchmod(sd, 0600 | S_ISUID); + ATF_REQUIRE_ERRNO(EINVAL, error == -1); + + umask(0022); + error = fchmod(sd, 0766); + ATF_REQUIRE(error == 0); + + error = bind(sd, (struct sockaddr *)&sun, sizeof(sun)); + ATF_REQUIRE(error == 0); + + error = stat(sun.sun_path, &sb); + ATF_REQUIRE(error == 0); + ATF_REQUIRE_MSG((sb.st_mode & 0777) == 0744, + "sb.st_mode = %o", sb.st_mode); + + error = fchmod(sd, 0666); + ATF_REQUIRE_ERRNO(EINVAL, error == -1); + + ATF_REQUIRE(close(sd) == 0); +} + ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, basic); ATF_TP_ADD_TC(tp, one2many); ATF_TP_ADD_TC(tp, event); ATF_TP_ADD_TC(tp, selfgetpeername); + ATF_TP_ADD_TC(tp, fchmod); return (atf_no_error()); }