Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ docs:
.PHONY: clean
clean:
rm -rf bin/ src/*.o src/*.gcno src/*.gcda *.gcov
$(MAKE) -C test clean
$(MAKE) -C docs clean

.PHONY: install install.bin install.crio install.podman podman crio
Expand Down
2 changes: 1 addition & 1 deletion src/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#define CONFIG_H

#define BUF_SIZE 8192
#define STDIO_BUF_SIZE 8192
#define DEF_STDOUT_BUF_SIZE 65536
#define CONN_SOCK_BUF_SIZE 32768
#define DEFAULT_SOCKET_PATH "/var/run/crio"
#define WIN_RESIZE_EVENT 1
Expand Down
23 changes: 19 additions & 4 deletions src/conmon.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ int main(int argc, char *argv[])
umask(DEFAULT_UMASK);
_cleanup_gerror_ GError *err = NULL;
char buf[BUF_SIZE];
int num_read;
ssize_t num_read;
_cleanup_close_ int dev_null_r_cleanup = -1;
_cleanup_close_ int dev_null_w_cleanup = -1;
_cleanup_close_ int dummyfd = -1;
Expand All @@ -66,7 +66,7 @@ int main(int argc, char *argv[])
/* Block for an initial write to the start pipe before
spawning any children or exiting, to ensure the
parent can put us in the right cgroup. */
num_read = read(start_pipe_fd, buf, BUF_SIZE);
num_read = read(start_pipe_fd, buf, sizeof(buf));
if (num_read < 0) {
pexit("start-pipe read failed");
}
Expand Down Expand Up @@ -165,6 +165,10 @@ int main(int argc, char *argv[])
pexit("Failed to create !terminal stdin pipe");

mainfd_stdin = fds[1];
ret = fcntl(mainfd_stdin, F_GETPIPE_SZ);
if (ret < 0)
pexit("main stdin pipe size determination failed");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit the fcntl should never fail, since we've just created the pipe and it will just be valid, but could we maybe default to DEF_STDOUT_BUF_SIZE also in this case and also for mainfd_stdout_size below? It seems to be more fail-safe way than simply exiting.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this one seems to be way brutal, just warning and mainfd_stdin_size = DEF_STDOUT_BUF_SIZE; would do it with grace it seems.

mainfd_stdin_size = (size_t)ret;
workerfd_stdin = fds[0];

if (g_unix_set_fd_nonblocking(mainfd_stdin, TRUE, NULL) == FALSE)
Expand All @@ -175,6 +179,10 @@ int main(int argc, char *argv[])
pexit("Failed to create !terminal stdout pipe");

mainfd_stdout = fds[0];
ret = fcntl(mainfd_stdout, F_GETPIPE_SZ);
if (ret < 0)
pexit("main stdout pipe size determination failed");
mainfd_stdout_size = (size_t)ret;
workerfd_stdout = fds[1];
}

Expand All @@ -194,6 +202,13 @@ int main(int argc, char *argv[])
pexit("Failed to create stderr pipe");

mainfd_stderr = fds[0];
ret = fcntl(mainfd_stderr, F_GETPIPE_SZ);
if (ret < 0)
pexit("main stderr pipe size determination failed");
mainfd_stderr_size = (size_t)ret;
if ((mainfd_stdout >= 0) && (mainfd_stderr_size != mainfd_stdout_size)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why do we care about the sizes to be the same?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never seen these being different in real life. As a matter of fact I'd remove the whole condition as it never triggers.

nwarn("main stderr and stdout pipe sizes don't match");
}
workerfd_stderr = fds[1];

GPtrArray *runtime_argv = configure_runtime_args(csname);
Expand Down Expand Up @@ -282,7 +297,7 @@ int main(int argc, char *argv[])
if (opt_attach) {
if (start_pipe_fd > 0) {
ndebug("exec with attach is waiting for start message from parent");
num_read = read(start_pipe_fd, buf, BUF_SIZE);
num_read = read(start_pipe_fd, buf, sizeof(buf));
if (num_read < 0) {
_pexit("start-pipe read failed");
}
Expand Down Expand Up @@ -370,7 +385,7 @@ int main(int argc, char *argv[])
* Read from container stderr for any error and send it to parent
* We send -1 as pid to signal to parent that create container has failed.
*/
num_read = read(mainfd_stderr, buf, BUF_SIZE - 1);
num_read = read(mainfd_stderr, buf, sizeof(buf) - 1);
const char *error_msg = NULL;
if (num_read > 0) {
buf[num_read] = '\0';
Expand Down
39 changes: 27 additions & 12 deletions src/conn_sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ static char *setup_socket(int *fd, const char *path);
setup_attach_socket() is responsible for setting the correct remote FD and pushing it onto the queue.
*/
static struct local_sock_s local_mainfd_stdin = {&mainfd_stdin, true, NULL, "container stdin", NULL};
struct remote_sock_s remote_attach_sock = {
static struct remote_sock_s remote_attach_sock = {
SOCK_TYPE_CONSOLE, /* sock_type */
-1, /* fd */
&local_mainfd_stdin, /* dest */
Expand Down Expand Up @@ -349,27 +349,42 @@ char *socket_parent_dir(gboolean use_full_attach_path, size_t desired_len)
return base_path;
}


void schedule_main_stdin_write()
{
schedule_local_sock_write(&local_mainfd_stdin);
}

void write_back_to_remote_consoles(char *buf, int len)
struct write_console_sock_user_data {
stdpipe_t pipe;
char *buf;
size_t buflen;
};

static void write_console_sock(gpointer data, G_GNUC_UNUSED gpointer user_data)
{
if (local_mainfd_stdin.readers == NULL)
struct remote_sock_s *remote_sock = (struct remote_sock_s *)data;
if (!remote_sock->writable)
return;

for (int i = local_mainfd_stdin.readers->len; i > 0; i--) {
struct remote_sock_s *remote_sock = g_ptr_array_index(local_mainfd_stdin.readers, i - 1);

if (remote_sock->writable && write_all(remote_sock->fd, buf, len) < 0) {
nwarn("Failed to write to remote console socket");
remote_sock_shutdown(remote_sock, SHUT_WR);
}
struct write_console_sock_user_data *wcsud = (struct write_console_sock_user_data *)user_data;
struct iovec iov[2] = {{&wcsud->pipe, 1}, {wcsud->buf, wcsud->buflen}};
writev_iov_t wviov = {2, 2, iov};
if (writev_all(remote_sock->fd, &wviov) < 0) {
nwarn("Failed to write to remote console socket");
remote_sock_shutdown(remote_sock, SHUT_WR);
}
}

void write_back_to_remote_consoles(stdpipe_t pipe, char *buf, size_t buflen)
{
if (local_mainfd_stdin.readers == NULL)
return;
GPtrArray *readers_copy = g_ptr_array_copy(local_mainfd_stdin.readers, NULL, NULL);
g_ptr_array_set_free_func(readers_copy, NULL);
struct write_console_sock_user_data wcsud = {pipe, buf, buflen};
g_ptr_array_foreach(readers_copy, write_console_sock, &wcsud);
g_ptr_array_free(readers_copy, true);
}

/* Internal */
static gboolean attach_cb(int fd, G_GNUC_UNUSED GIOCondition condition, gpointer user_data)
{
Expand Down
3 changes: 2 additions & 1 deletion src/conn_sock.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <glib.h> /* gboolean */
#include "config.h" /* CONN_SOCK_BUF_SIZE */
#include "utils.h" /* stdpipe_t */

#define SOCK_TYPE_CONSOLE 1
#define SOCK_TYPE_NOTIFY 2
Expand Down Expand Up @@ -52,7 +53,7 @@ char *setup_seccomp_socket(const char *socket);
char *setup_attach_socket(void);
void setup_notify_socket(char *);
void schedule_main_stdin_write();
void write_back_to_remote_consoles(char *buf, int len);
void write_back_to_remote_consoles(stdpipe_t pipe, char *buf, size_t buflen);
void close_all_readers();

#endif // CONN_SOCK_H
Loading
Loading