SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

terminal-util: various minor modernizations

Browse Source 
Various fixes:

1. Adds O_CLOEXEC for two socketpair()s where we forgot it.

2. Uses FORK_WAIT instead of manual wait_for_terminate_and_check()
   invocations.

3. Prefix opaque NULL/0 arguments with comments what they are.

4. Add a banch of assert()s, and change flag validation in
   open_terminal() to be assert() (since flags mistakes are programming
   errors, not runtime errors).
This commit is contained in:
Lennart Poettering
2024-10-30 22:15:56 +01:00
parent 954dd5242b
commit fbd2679f66
1 changed files with 50 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/terminal-util.c
+50 -44
View File
@@ -323,7 +323,6 @@ int show_menu(char **x, unsigned n_columns, unsigned width, unsigned percentage)
int open_terminal(const char *name, int mode) {
_cleanup_close_ int fd = -EBADF;
unsigned c = 0;
/*
* If a TTY is in the process of being closed opening it might cause EIO. This is horribly awful, but
@@ -333,10 +332,9 @@ int open_terminal(const char *name, int mode) {
* https://bugs.launchpad.net/ubuntu/+source/linux/+bug/554172/comments/245
*/
if ((mode & (O_CREAT|O_PATH|O_DIRECTORY|O_TMPFILE)) != 0)
return -EINVAL;
assert((mode & (O_CREAT|O_PATH|O_DIRECTORY|O_TMPFILE)) == 0);
for (;;) {
for (unsigned c = 0;; c++) {
fd = open(name, mode, 0);
if (fd >= 0)
break;
@@ -346,10 +344,9 @@ int open_terminal(const char *name, int mode) {
/* Max 1s in total */
if (c >= 20)
return -errno;
return -EIO;
(void) usleep_safe(50 * USEC_PER_MSEC);
c++;
}
if (!isatty_safe(fd))
@@ -1296,16 +1293,16 @@ int ptsname_malloc(int fd, char **ret) {
}
}
int openpt_allocate(int flags, char **ret_slave) {
int openpt_allocate(int flags, char **ret_peer_path) {
_cleanup_close_ int fd = -EBADF;
_cleanup_free_ char *p = NULL;
int r;
fd = posix_openpt(flags|O_NOCTTY|O_CLOEXEC);
if (fd < 0)
return -errno;
if (ret_slave) {
_cleanup_free_ char *p = NULL;
if (ret_peer_path) {
r = ptsname_malloc(fd, &p);
if (r < 0)
return r;
@@ -1317,20 +1314,22 @@ int openpt_allocate(int flags, char **ret_slave) {
if (unlockpt(fd) < 0)
return -errno;
if (ret_slave)
*ret_slave = TAKE_PTR(p);
if (ret_peer_path)
*ret_peer_path = TAKE_PTR(p);
return TAKE_FD(fd);
}
static int ptsname_namespace(int pty, char **ret) {
int no = -1, r;
int no = -1;
assert(pty >= 0);
assert(ret);
/* Like ptsname(), but doesn't assume that the path is
* accessible in the local namespace. */
r = ioctl(pty, TIOCGPTN, &no);
if (r < 0)
if (ioctl(pty, TIOCGPTN, &no) < 0)
return -errno;
if (no < 0)
@@ -1342,10 +1341,9 @@ static int ptsname_namespace(int pty, char **ret) {
return 0;
}
int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) {
int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_peer_path) {
_cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, usernsfd = -EBADF, rootfd = -EBADF, fd = -EBADF;
_cleanup_close_pair_ int pair[2] = EBADF_PAIR;
pid_t child;
int r;
assert(pid > 0);
@@ -1354,17 +1352,27 @@ int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) {
if (r < 0)
return r;
if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0)
if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pair) < 0)
return -errno;
r = namespace_fork("(sd-openptns)", "(sd-openpt)", NULL, 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL,
pidnsfd, mntnsfd, -1, usernsfd, rootfd, &child);
r = namespace_fork(
"(sd-openptns)",
"(sd-openpt)",
/* except_fds= */ NULL,
/* n_except_fds= */ 0,
FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL|FORK_WAIT,
pidnsfd,
mntnsfd,
/* netns_fd= */ -EBADF,
usernsfd,
rootfd,
/* ret_pid= */ NULL);
if (r < 0)
return r;
if (r == 0) {
pair[0] = safe_close(pair[0]);
fd = openpt_allocate(flags, NULL);
fd = openpt_allocate(flags, /* ret_peer_path= */ NULL);
if (fd < 0)
_exit(EXIT_FAILURE);
@@ -1376,18 +1384,12 @@ int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) {
pair[1] = safe_close(pair[1]);
r = wait_for_terminate_and_check("(sd-openptns)", child, 0);
if (r < 0)
return r;
if (r != EXIT_SUCCESS)
return -EIO;
fd = receive_one_fd(pair[0], 0);
if (fd < 0)
return fd;
if (ret_slave) {
r = ptsname_namespace(fd, ret_slave);
if (ret_peer_path) {
r = ptsname_namespace(fd, ret_peer_path);
if (r < 0)
return r;
}
@@ -1398,30 +1400,40 @@ int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) {
int open_terminal_in_namespace(pid_t pid, const char *name, int mode) {
_cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, usernsfd = -EBADF, rootfd = -EBADF;
_cleanup_close_pair_ int pair[2] = EBADF_PAIR;
pid_t child;
int r;
r = namespace_open(pid, &pidnsfd, &mntnsfd, /* ret_netns_fd = */ NULL, &usernsfd, &rootfd);
assert(pid > 0);
assert(name);
r = namespace_open(pid, &pidnsfd, &mntnsfd, /* ret_netns_fd= */ NULL, &usernsfd, &rootfd);
if (r < 0)
return r;
if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0)
if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pair) < 0)
return -errno;
r = namespace_fork("(sd-terminalns)", "(sd-terminal)", NULL, 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL,
pidnsfd, mntnsfd, -1, usernsfd, rootfd, &child);
r = namespace_fork(
"(sd-terminalns)",
"(sd-terminal)",
/* except_fds= */ NULL,
/* n_except_fds= */ 0,
FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL|FORK_WAIT,
pidnsfd,
mntnsfd,
/* netnsd_fd= */ -EBADF,
usernsfd,
rootfd,
/* ret_pid= */ NULL);
if (r < 0)
return r;
if (r == 0) {
int master;
pair[0] = safe_close(pair[0]);
master = open_terminal(name, mode|O_NOCTTY|O_CLOEXEC);
if (master < 0)
int pty_fd = open_terminal(name, mode|O_NOCTTY|O_CLOEXEC);
if (pty_fd < 0)
_exit(EXIT_FAILURE);
if (send_one_fd(pair[1], master, 0) < 0)
if (send_one_fd(pair[1], pty_fd, 0) < 0)
_exit(EXIT_FAILURE);
_exit(EXIT_SUCCESS);
@@ -1429,12 +1441,6 @@ int open_terminal_in_namespace(pid_t pid, const char *name, int mode) {
pair[1] = safe_close(pair[1]);
r = wait_for_terminate_and_check("(sd-terminalns)", child, 0);
if (r < 0)
return r;
if (r != EXIT_SUCCESS)
return -EIO;
return receive_one_fd(pair[0], 0);
}