git: bfd03046d187 - main - unix: Add support for atomically setting the socket mode
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 03 Nov 2024 16:48:08 UTC
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=bfd03046d18776ea70785ca1ef36dfc60822de3b
commit bfd03046d18776ea70785ca1ef36dfc60822de3b
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2024-11-03 14:39:32 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
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 <sys/_types.h>
+
/* 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 <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());
}