Date: Tue, 3 Dec 2024 02:17:11 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: da44138dc2ad - stable/14 - unix: Add support for atomically setting the socket mode Message-ID: <202412030217.4B32HBJ9070737@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=da44138dc2adefb698bd34c9cff73e1a5588ef26 commit da44138dc2adefb698bd34c9cff73e1a5588ef26 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2024-11-03 14:39:32 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2024-12-03 01:03:26 +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 (cherry picked from commit bfd03046d18776ea70785ca1ef36dfc60822de3b) --- lib/libc/sys/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 | 4 ++++ sys/sys/unpcb.h | 1 + tests/sys/kern/unix_dgram.c | 41 ++++++++++++++++++++++++++++++++++++++++- 7 files changed, 108 insertions(+), 5 deletions(-) diff --git a/lib/libc/sys/chmod.2 b/lib/libc/sys/chmod.2 index 7bcf19d50a1d..b3cb8aad0230 100644 --- a/lib/libc/sys/chmod.2 +++ b/lib/libc/sys/chmod.2 @@ -27,7 +27,7 @@ .\" .\" @(#)chmod.2 8.1 (Berkeley) 6/4/93 .\" -.Dd March 30, 2021 +.Dd October 31, 2024 .Dt CHMOD 2 .Os .Sh NAME @@ -216,6 +216,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 c055ed230190..7b8d33273c0a 100644 --- a/share/man/man4/unix.4 +++ b/share/man/man4/unix.4 @@ -27,7 +27,7 @@ .\" .\" @(#)unix.4 8.1 (Berkeley) 6/9/93 .\" -.Dd June 24, 2022 +.Dd October 31, 2024 .Dt UNIX 4 .Os .Sh NAME @@ -79,6 +79,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 @@ -451,6 +460,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 45b458f81f19..58891b0de000 100644 --- a/sys/kern/sys_socket.c +++ b/sys/kern/sys_socket.c @@ -93,6 +93,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; @@ -107,7 +108,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, @@ -356,6 +357,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 87b82763b5f1..78469adcd84d 100644 --- a/sys/kern/uipc_usrreq.c +++ b/sys/kern/uipc_usrreq.c @@ -511,6 +511,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(); @@ -553,6 +554,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); @@ -585,6 +587,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); @@ -617,7 +620,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); @@ -729,6 +732,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) { @@ -3352,6 +3376,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 = { @@ -3376,6 +3401,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 = { @@ -3407,6 +3433,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 66b194ff1b9a..3f1e2f897cac 100644 --- a/sys/sys/protosw.h +++ b/sys/sys/protosw.h @@ -34,6 +34,7 @@ #ifndef _SYS_PROTOSW_H_ #define _SYS_PROTOSW_H_ +#include <sys/_types.h> #include <sys/queue.h> /* Forward declare these structures referenced from prototypes below. */ @@ -104,6 +105,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 */ @@ -144,6 +147,7 @@ struct protosw { pr_flush_t *pr_flush; /* XXXGL: merge with pr_shutdown_t! */ 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 8549ea279c16..0deb9b4690c7 100644 --- a/sys/sys/unpcb.h +++ b/sys/sys/unpcb.h @@ -95,6 +95,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 <sys/time.h> #include <sys/event.h> #include <sys/ioctl.h> #include <sys/select.h> #include <sys/socket.h> +#include <sys/stat.h> #include <sys/sysctl.h> +#include <sys/time.h> #include <sys/un.h> + #include <aio.h> #include <errno.h> #include <fcntl.h> @@ -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()); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202412030217.4B32HBJ9070737>