diff options
Diffstat (limited to '0014-libxl-Fix-handling-XenStore-errors-in-device-creatio.patch')
| -rw-r--r-- | 0014-libxl-Fix-handling-XenStore-errors-in-device-creatio.patch | 191 |
1 files changed, 191 insertions, 0 deletions
diff --git a/0014-libxl-Fix-handling-XenStore-errors-in-device-creatio.patch b/0014-libxl-Fix-handling-XenStore-errors-in-device-creatio.patch new file mode 100644 index 0000000..ac28521 --- /dev/null +++ b/0014-libxl-Fix-handling-XenStore-errors-in-device-creatio.patch @@ -0,0 +1,191 @@ +From 8271f0e8f23b63199caf0edcfe85ebc1c1412d1b Mon Sep 17 00:00:00 2001 +From: Demi Marie Obenour <demi@invisiblethingslab.com> +Date: Tue, 21 May 2024 10:23:52 +0200 +Subject: [PATCH 14/56] libxl: Fix handling XenStore errors in device creation + +If xenstored runs out of memory it is possible for it to fail operations +that should succeed. libxl wasn't robust against this, and could fail +to ensure that the TTY path of a non-initial console was created and +read-only for guests. This doesn't qualify for an XSA because guests +should not be able to run xenstored out of memory, but it still needs to +be fixed. + +Add the missing error checks to ensure that all errors are properly +handled and that at no point can a guest make the TTY path of its +frontend directory writable. + +Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com> +Reviewed-by: Juergen Gross <jgross@suse.com> +master commit: 531d3bea5e9357357eaf6d40f5784a1b4c29b910 +master date: 2024-05-11 00:13:43 +0100 +--- + tools/libs/light/libxl_console.c | 11 ++--- + tools/libs/light/libxl_device.c | 72 ++++++++++++++++++++------------ + tools/libs/light/libxl_xshelp.c | 13 ++++-- + 3 files changed, 60 insertions(+), 36 deletions(-) + +diff --git a/tools/libs/light/libxl_console.c b/tools/libs/light/libxl_console.c +index cd7412a327..a563c9d3c7 100644 +--- a/tools/libs/light/libxl_console.c ++++ b/tools/libs/light/libxl_console.c +@@ -351,11 +351,10 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid, + flexarray_append(front, "protocol"); + flexarray_append(front, LIBXL_XENCONSOLE_PROTOCOL); + } +- libxl__device_generic_add(gc, XBT_NULL, device, +- libxl__xs_kvs_of_flexarray(gc, back), +- libxl__xs_kvs_of_flexarray(gc, front), +- libxl__xs_kvs_of_flexarray(gc, ro_front)); +- rc = 0; ++ rc = libxl__device_generic_add(gc, XBT_NULL, device, ++ libxl__xs_kvs_of_flexarray(gc, back), ++ libxl__xs_kvs_of_flexarray(gc, front), ++ libxl__xs_kvs_of_flexarray(gc, ro_front)); + out: + return rc; + } +@@ -665,6 +664,8 @@ int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid, + */ + if (!val) val = "/NO-SUCH-PATH"; + channelinfo->u.pty.path = strdup(val); ++ if (channelinfo->u.pty.path == NULL) ++ abort(); + break; + default: + break; +diff --git a/tools/libs/light/libxl_device.c b/tools/libs/light/libxl_device.c +index 13da6e0573..3035501f2c 100644 +--- a/tools/libs/light/libxl_device.c ++++ b/tools/libs/light/libxl_device.c +@@ -177,8 +177,13 @@ int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t, + ro_frontend_perms[1].perms = backend_perms[1].perms = XS_PERM_READ; + + retry_transaction: +- if (create_transaction) ++ if (create_transaction) { + t = xs_transaction_start(ctx->xsh); ++ if (t == XBT_NULL) { ++ LOGED(ERROR, device->domid, "xs_transaction_start failed"); ++ return ERROR_FAIL; ++ } ++ } + + /* FIXME: read frontend_path and check state before removing stuff */ + +@@ -195,42 +200,55 @@ retry_transaction: + if (rc) goto out; + } + +- /* xxx much of this function lacks error checks! */ +- + if (fents || ro_fents) { +- xs_rm(ctx->xsh, t, frontend_path); +- xs_mkdir(ctx->xsh, t, frontend_path); ++ if (!xs_rm(ctx->xsh, t, frontend_path) && errno != ENOENT) ++ goto out; ++ if (!xs_mkdir(ctx->xsh, t, frontend_path)) ++ goto out; + /* Console 0 is a special case. It doesn't use the regular PV + * state machine but also the frontend directory has + * historically contained other information, such as the + * vnc-port, which we don't want the guest fiddling with. + */ + if ((device->kind == LIBXL__DEVICE_KIND_CONSOLE && device->devid == 0) || +- (device->kind == LIBXL__DEVICE_KIND_VUART)) +- xs_set_permissions(ctx->xsh, t, frontend_path, +- ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms)); +- else +- xs_set_permissions(ctx->xsh, t, frontend_path, +- frontend_perms, ARRAY_SIZE(frontend_perms)); +- xs_write(ctx->xsh, t, GCSPRINTF("%s/backend", frontend_path), +- backend_path, strlen(backend_path)); +- if (fents) +- libxl__xs_writev_perms(gc, t, frontend_path, fents, +- frontend_perms, ARRAY_SIZE(frontend_perms)); +- if (ro_fents) +- libxl__xs_writev_perms(gc, t, frontend_path, ro_fents, +- ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms)); ++ (device->kind == LIBXL__DEVICE_KIND_VUART)) { ++ if (!xs_set_permissions(ctx->xsh, t, frontend_path, ++ ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms))) ++ goto out; ++ } else { ++ if (!xs_set_permissions(ctx->xsh, t, frontend_path, ++ frontend_perms, ARRAY_SIZE(frontend_perms))) ++ goto out; ++ } ++ if (!xs_write(ctx->xsh, t, GCSPRINTF("%s/backend", frontend_path), ++ backend_path, strlen(backend_path))) ++ goto out; ++ if (fents) { ++ rc = libxl__xs_writev_perms(gc, t, frontend_path, fents, ++ frontend_perms, ARRAY_SIZE(frontend_perms)); ++ if (rc) goto out; ++ } ++ if (ro_fents) { ++ rc = libxl__xs_writev_perms(gc, t, frontend_path, ro_fents, ++ ro_frontend_perms, ARRAY_SIZE(ro_frontend_perms)); ++ if (rc) goto out; ++ } + } + + if (bents) { + if (!libxl_only) { +- xs_rm(ctx->xsh, t, backend_path); +- xs_mkdir(ctx->xsh, t, backend_path); +- xs_set_permissions(ctx->xsh, t, backend_path, backend_perms, +- ARRAY_SIZE(backend_perms)); +- xs_write(ctx->xsh, t, GCSPRINTF("%s/frontend", backend_path), +- frontend_path, strlen(frontend_path)); +- libxl__xs_writev(gc, t, backend_path, bents); ++ if (!xs_rm(ctx->xsh, t, backend_path) && errno != ENOENT) ++ goto out; ++ if (!xs_mkdir(ctx->xsh, t, backend_path)) ++ goto out; ++ if (!xs_set_permissions(ctx->xsh, t, backend_path, backend_perms, ++ ARRAY_SIZE(backend_perms))) ++ goto out; ++ if (!xs_write(ctx->xsh, t, GCSPRINTF("%s/frontend", backend_path), ++ frontend_path, strlen(frontend_path))) ++ goto out; ++ rc = libxl__xs_writev(gc, t, backend_path, bents); ++ if (rc) goto out; + } + + /* +@@ -276,7 +294,7 @@ retry_transaction: + out: + if (create_transaction && t) + libxl__xs_transaction_abort(gc, &t); +- return rc; ++ return rc != 0 ? rc : ERROR_FAIL; + } + + typedef struct { +diff --git a/tools/libs/light/libxl_xshelp.c b/tools/libs/light/libxl_xshelp.c +index 751cd942d9..a6e34ab10f 100644 +--- a/tools/libs/light/libxl_xshelp.c ++++ b/tools/libs/light/libxl_xshelp.c +@@ -60,10 +60,15 @@ int libxl__xs_writev_perms(libxl__gc *gc, xs_transaction_t t, + for (i = 0; kvs[i] != NULL; i += 2) { + path = GCSPRINTF("%s/%s", dir, kvs[i]); + if (path && kvs[i + 1]) { +- int length = strlen(kvs[i + 1]); +- xs_write(ctx->xsh, t, path, kvs[i + 1], length); +- if (perms) +- xs_set_permissions(ctx->xsh, t, path, perms, num_perms); ++ size_t length = strlen(kvs[i + 1]); ++ if (length > UINT_MAX) ++ return ERROR_FAIL; ++ if (!xs_write(ctx->xsh, t, path, kvs[i + 1], length)) ++ return ERROR_FAIL; ++ if (perms) { ++ if (!xs_set_permissions(ctx->xsh, t, path, perms, num_perms)) ++ return ERROR_FAIL; ++ } + } + } + return 0; +-- +2.45.2 + |