qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio

qemu-ga: obey LISTEN_PID when using systemd socket activation

qemu-ga's socket activation support was not obeying the LISTEN_PID
environment variable, which avoids that a process uses a socket-activation
file descriptor meant for its parent.

Mess can for example ensue if a process forks a children before consuming
the socket-activation file descriptor and therefore setting O_CLOEXEC
on it.

Luckily, qemu-nbd also got socket activation code, and its copy does
support LISTEN_PID. Some extra fixups are needed to ensure that the
code can be used for both, but that's what this patch does. The
main change is to replace get_listen_fds's "consume" argument with
the FIRST_SOCKET_ACTIVATION_FD macro from the qemu-nbd code.

Cc: "Richard W.M. Jones" <rjones@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

+125 -130
+26
include/qemu/systemd.h
··· 1 + /* 2 + * systemd socket activation support 3 + * 4 + * Copyright 2017 Red Hat, Inc. and/or its affiliates 5 + * 6 + * Authors: 7 + * Richard W.M. Jones <rjones@redhat.com> 8 + * 9 + * This work is licensed under the terms of the GNU GPL, version 2 or later. 10 + * See the COPYING file in the top-level directory. 11 + */ 12 + 13 + #ifndef QEMU_SYSTEMD_H 14 + #define QEMU_SYSTEMD_H 1 15 + 16 + #define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */ 17 + 18 + /* 19 + * Check if socket activation was requested via use of the 20 + * LISTEN_FDS and LISTEN_PID environment variables. 21 + * 22 + * Returns 0 if no socket activation, or the number of FDs. 23 + */ 24 + unsigned int check_socket_activation(void); 25 + 26 + #endif
+8 -92
qemu-nbd.c
··· 28 28 #include "qemu/config-file.h" 29 29 #include "qemu/bswap.h" 30 30 #include "qemu/log.h" 31 + #include "qemu/systemd.h" 31 32 #include "block/snapshot.h" 32 33 #include "qapi/util.h" 33 34 #include "qapi/qmp/qstring.h" ··· 474 475 } 475 476 } 476 477 477 - #define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */ 478 - 479 - #ifndef _WIN32 480 - /* 481 - * Check if socket activation was requested via use of the 482 - * LISTEN_FDS and LISTEN_PID environment variables. 483 - * 484 - * Returns 0 if no socket activation, or the number of FDs. 485 - */ 486 - static unsigned int check_socket_activation(void) 487 - { 488 - const char *s; 489 - unsigned long pid; 490 - unsigned long nr_fds; 491 - unsigned int i; 492 - int fd; 493 - int err; 494 - 495 - s = getenv("LISTEN_PID"); 496 - if (s == NULL) { 497 - return 0; 498 - } 499 - err = qemu_strtoul(s, NULL, 10, &pid); 500 - if (err) { 501 - if (verbose) { 502 - fprintf(stderr, "malformed %s environment variable (ignored)\n", 503 - "LISTEN_PID"); 504 - } 505 - return 0; 506 - } 507 - if (pid != getpid()) { 508 - if (verbose) { 509 - fprintf(stderr, "%s was not for us (ignored)\n", 510 - "LISTEN_PID"); 511 - } 512 - return 0; 513 - } 514 - 515 - s = getenv("LISTEN_FDS"); 516 - if (s == NULL) { 517 - return 0; 518 - } 519 - err = qemu_strtoul(s, NULL, 10, &nr_fds); 520 - if (err) { 521 - if (verbose) { 522 - fprintf(stderr, "malformed %s environment variable (ignored)\n", 523 - "LISTEN_FDS"); 524 - } 525 - return 0; 526 - } 527 - assert(nr_fds <= UINT_MAX); 528 - 529 - /* A limitation of current qemu-nbd is that it can only listen on 530 - * a single socket. When that limitation is lifted, we can change 531 - * this function to allow LISTEN_FDS > 1, and remove the assertion 532 - * in the main function below. 533 - */ 534 - if (nr_fds > 1) { 535 - error_report("qemu-nbd does not support socket activation with %s > 1", 536 - "LISTEN_FDS"); 537 - exit(EXIT_FAILURE); 538 - } 539 - 540 - /* So these are not passed to any child processes we might start. */ 541 - unsetenv("LISTEN_FDS"); 542 - unsetenv("LISTEN_PID"); 543 - 544 - /* So the file descriptors don't leak into child processes. */ 545 - for (i = 0; i < nr_fds; ++i) { 546 - fd = FIRST_SOCKET_ACTIVATION_FD + i; 547 - if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) { 548 - /* If we cannot set FD_CLOEXEC then it probably means the file 549 - * descriptor is invalid, so socket activation has gone wrong 550 - * and we should exit. 551 - */ 552 - error_report("Socket activation failed: " 553 - "invalid file descriptor fd = %d: %m", 554 - fd); 555 - exit(EXIT_FAILURE); 556 - } 557 - } 558 - 559 - return (unsigned int) nr_fds; 560 - } 561 - 562 - #else /* !_WIN32 */ 563 - static unsigned int check_socket_activation(void) 564 - { 565 - return 0; 566 - } 567 - #endif 568 - 569 478 /* 570 479 * Check socket parameters compatibility when socket activation is used. 571 480 */ ··· 890 799 bindto, port); 891 800 if (err_msg != NULL) { 892 801 error_report("%s", err_msg); 802 + exit(EXIT_FAILURE); 803 + } 804 + 805 + /* qemu-nbd can only listen on a single socket. */ 806 + if (socket_activation > 1) { 807 + error_report("qemu-nbd does not support socket activation with %s > 1", 808 + "LISTEN_FDS"); 893 809 exit(EXIT_FAILURE); 894 810 } 895 811 }
+13 -38
qga/main.c
··· 29 29 #include "qemu/bswap.h" 30 30 #include "qemu/help_option.h" 31 31 #include "qemu/sockets.h" 32 + #include "qemu/systemd.h" 32 33 #ifdef _WIN32 33 34 #include "qga/service-win32.h" 34 35 #include "qga/vss-win32.h" ··· 185 186 } 186 187 } 187 188 #endif 188 - 189 - /** 190 - * get_listen_fd: 191 - * @consume: true to prevent future calls from succeeding 192 - * 193 - * Fetch a listen file descriptor that was passed via systemd socket 194 - * activation. Use @consume to prevent child processes from thinking a file 195 - * descriptor was passed. 196 - * 197 - * Returns: file descriptor or -1 if no fd was passed 198 - */ 199 - static int get_listen_fd(bool consume) 200 - { 201 - #ifdef _WIN32 202 - return -1; /* no fd passing expected, unsetenv(3) not available */ 203 - #else 204 - const char *listen_fds = getenv("LISTEN_FDS"); 205 - int fd = STDERR_FILENO + 1; 206 - 207 - if (!listen_fds || strcmp(listen_fds, "1") != 0) { 208 - return -1; 209 - } 210 - 211 - if (consume) { 212 - unsetenv("LISTEN_FDS"); 213 - } 214 - 215 - qemu_set_cloexec(fd); 216 - return fd; 217 - #endif /* !_WIN32 */ 218 - } 219 189 220 190 static void usage(const char *cmd) 221 191 { ··· 1251 1221 return false; 1252 1222 } 1253 1223 1254 - static int run_agent(GAState *s, GAConfig *config) 1224 + static int run_agent(GAState *s, GAConfig *config, int socket_activation) 1255 1225 { 1256 1226 ga_state = s; 1257 1227 ··· 1333 1303 s->main_loop = g_main_loop_new(NULL, false); 1334 1304 1335 1305 if (!channel_init(ga_state, config->method, config->channel_path, 1336 - get_listen_fd(true))) { 1306 + socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) { 1337 1307 g_critical("failed to initialize guest agent channel"); 1338 1308 return EXIT_FAILURE; 1339 1309 } ··· 1357 1327 int ret = EXIT_SUCCESS; 1358 1328 GAState *s = g_new0(GAState, 1); 1359 1329 GAConfig *config = g_new0(GAConfig, 1); 1360 - int listen_fd; 1330 + int socket_activation; 1361 1331 1362 1332 config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; 1363 1333 ··· 1379 1349 config->method = g_strdup("virtio-serial"); 1380 1350 } 1381 1351 1382 - listen_fd = get_listen_fd(false); 1383 - if (listen_fd >= 0) { 1352 + socket_activation = check_socket_activation(); 1353 + if (socket_activation > 1) { 1354 + g_critical("qemu-ga only supports listening on one socket"); 1355 + ret = EXIT_FAILURE; 1356 + goto end; 1357 + } 1358 + if (socket_activation) { 1384 1359 SocketAddress *addr; 1385 1360 1386 1361 g_free(config->method); ··· 1388 1363 config->method = NULL; 1389 1364 config->channel_path = NULL; 1390 1365 1391 - addr = socket_local_address(listen_fd, NULL); 1366 + addr = socket_local_address(FIRST_SOCKET_ACTIVATION_FD, NULL); 1392 1367 if (addr) { 1393 1368 if (addr->type == SOCKET_ADDRESS_KIND_UNIX) { 1394 1369 config->method = g_strdup("unix-listen"); ··· 1433 1408 goto end; 1434 1409 } 1435 1410 1436 - ret = run_agent(s, config); 1411 + ret = run_agent(s, config, socket_activation); 1437 1412 1438 1413 end: 1439 1414 if (s->command_state) {
+1
util/Makefile.objs
··· 42 42 util-obj-y += qdist.o 43 43 util-obj-y += qht.o 44 44 util-obj-y += range.o 45 + util-obj-y += systemd.o
+77
util/systemd.c
··· 1 + /* 2 + * systemd socket activation support 3 + * 4 + * Copyright 2017 Red Hat, Inc. and/or its affiliates 5 + * 6 + * Authors: 7 + * Richard W.M. Jones <rjones@redhat.com> 8 + * 9 + * This work is licensed under the terms of the GNU GPL, version 2 or later. 10 + * See the COPYING file in the top-level directory. 11 + */ 12 + 13 + #include "qemu/osdep.h" 14 + #include "qemu/systemd.h" 15 + #include "qemu/cutils.h" 16 + #include "qemu/error-report.h" 17 + 18 + #ifndef _WIN32 19 + unsigned int check_socket_activation(void) 20 + { 21 + const char *s; 22 + unsigned long pid; 23 + unsigned long nr_fds; 24 + unsigned int i; 25 + int fd; 26 + int err; 27 + 28 + s = getenv("LISTEN_PID"); 29 + if (s == NULL) { 30 + return 0; 31 + } 32 + err = qemu_strtoul(s, NULL, 10, &pid); 33 + if (err) { 34 + return 0; 35 + } 36 + if (pid != getpid()) { 37 + return 0; 38 + } 39 + 40 + s = getenv("LISTEN_FDS"); 41 + if (s == NULL) { 42 + return 0; 43 + } 44 + err = qemu_strtoul(s, NULL, 10, &nr_fds); 45 + if (err) { 46 + return 0; 47 + } 48 + assert(nr_fds <= UINT_MAX); 49 + 50 + /* So these are not passed to any child processes we might start. */ 51 + unsetenv("LISTEN_FDS"); 52 + unsetenv("LISTEN_PID"); 53 + 54 + /* So the file descriptors don't leak into child processes. */ 55 + for (i = 0; i < nr_fds; ++i) { 56 + fd = FIRST_SOCKET_ACTIVATION_FD + i; 57 + if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) { 58 + /* If we cannot set FD_CLOEXEC then it probably means the file 59 + * descriptor is invalid, so socket activation has gone wrong 60 + * and we should exit. 61 + */ 62 + error_report("Socket activation failed: " 63 + "invalid file descriptor fd = %d: %m", 64 + fd); 65 + exit(EXIT_FAILURE); 66 + } 67 + } 68 + 69 + return (unsigned int) nr_fds; 70 + } 71 + 72 + #else /* !_WIN32 */ 73 + unsigned int check_socket_activation(void) 74 + { 75 + return 0; 76 + } 77 + #endif