Ensure that libnv can be used when kern.trap_enotcap=1.

libnv used fcntl(fd, F_GETFL) to test whether fd is a valid file
descriptor.  Aside from being racy, this check requires CAP_FCNTL
rights on fd.  Instead, use fcntl(fd, F_GETFD), which does not require
any capability rights.

Also remove some redundant fd_is_valid() checks to avoid extra system
calls; in many cases we were performing this check immediately before
dup()ing the descriptor.

Reviewed by:	cem, oshogbo (previous version)
MFC after:	2 weeks
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D17963
This commit is contained in:
Mark Johnston 2018-11-13 20:07:55 +00:00
parent 8643808a68
commit 991666adc7
4 changed files with 42 additions and 26 deletions

View File

@ -34,6 +34,15 @@
#ifndef _COMMON_IMPL_H_
#define _COMMON_IMPL_H_
#define fd_is_valid(fd) (fcntl((fd), F_GETFL) != -1 || errno != EBADF)
#include <errno.h>
#include <fcntl.h>
#include <stdbool.h>
static inline bool
fd_is_valid(int fd)
{
return (fcntl(fd, F_GETFD) != -1 || errno != EBADF);
}
#endif /* !_COMMON_IMPL_H_ */

View File

@ -66,11 +66,6 @@ msghdr_add_fd(struct cmsghdr *cmsg, int fd)
PJDLOG_ASSERT(fd >= 0);
if (!fd_is_valid(fd)) {
errno = EBADF;
return (-1);
}
cmsg->cmsg_level = SOL_SOCKET;
cmsg->cmsg_type = SCM_RIGHTS;
cmsg->cmsg_len = CMSG_LEN(sizeof(fd));

View File

@ -306,15 +306,12 @@ parent(int sock)
CHECK(name == NULL);
}
int
main(void)
static void
send_nvlist(void)
{
int status, socks[2];
pid_t pid;
printf("1..134\n");
fflush(stdout);
if (socketpair(PF_UNIX, SOCK_STREAM, 0, socks) < 0)
err(1, "socketpair() failed");
pid = fork();
@ -326,7 +323,7 @@ main(void)
/* Child. */
close(socks[0]);
child(socks[1]);
return (0);
_exit(0);
default:
/* Parent. */
close(socks[1]);
@ -336,6 +333,35 @@ main(void)
if (waitpid(pid, &status, 0) < 0)
err(1, "waitpid() failed");
}
static void
send_closed_fd(void)
{
nvlist_t *nvl;
int error, socks[2];
if (socketpair(PF_UNIX, SOCK_STREAM, 0, socks) < 0)
err(1, "socketpair() failed");
nvl = nvlist_create(0);
nvlist_add_descriptor(nvl, "fd", 12345);
error = nvlist_error(nvl);
CHECK(error == EBADF);
error = nvlist_send(socks[1], nvl);
CHECK(error != 0 && errno == EBADF);
}
int
main(void)
{
printf("1..136\n");
fflush(stdout);
send_nvlist();
send_closed_fd();
return (0);
}

View File

@ -1276,11 +1276,6 @@ nvpair_create_descriptor(const char *name, int value)
{
nvpair_t *nvp;
if (value < 0 || !fd_is_valid(value)) {
ERRNO_SET(EBADF);
return (NULL);
}
value = fcntl(value, F_DUPFD_CLOEXEC, 0);
if (value < 0)
return (NULL);
@ -1517,11 +1512,6 @@ nvpair_create_descriptor_array(const char *name, const int *value,
if (value[ii] == -1) {
fds[ii] = -1;
} else {
if (!fd_is_valid(value[ii])) {
ERRNO_SET(EBADF);
goto fail;
}
fds[ii] = fcntl(value[ii], F_DUPFD_CLOEXEC, 0);
if (fds[ii] == -1)
goto fail;
@ -2035,10 +2025,6 @@ nvpair_append_descriptor_array(nvpair_t *nvp, const int value)
NVPAIR_ASSERT(nvp);
PJDLOG_ASSERT(nvp->nvp_type == NV_TYPE_DESCRIPTOR_ARRAY);
if (value < 0 || !fd_is_valid(value)) {
ERRNO_SET(EBADF);
return -1;
}
fd = fcntl(value, F_DUPFD_CLOEXEC, 0);
if (fd == -1) {
return (-1);