Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2020-04-15-tag' into staging
qemu-ga patch queue for hard-freeze
* enforce 48MB limit for guest-file-read to avoid memory allocation
failures
# gpg: Signature made Wed 15 Apr 2020 15:23:48 BST
# gpg: using RSA key CEACC9E15534EBABB82D3FA03353C9CEF108B584
# gpg: issuer "mdroth@linux.vnet.ibm.com"
# gpg: Good signature from "Michael Roth <flukshun@gmail.com>" [full]
# gpg: aka "Michael Roth <mdroth@utexas.edu>" [full]
# gpg: aka "Michael Roth <mdroth@linux.vnet.ibm.com>" [full]
# Primary key fingerprint: CEAC C9E1 5534 EBAB B82D 3FA0 3353 C9CE F108 B584
* remotes/mdroth/tags/qga-pull-2020-04-15-tag:
qga: Restrict guest-file-read count to 48 MB to avoid crashes
qga: Extract qmp_guest_file_read() to common commands.c
qga: Extract guest_file_handle_find() to commands-common.h
Revert "prevent crash when executing guest-file-read with large count"
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
diff --git a/qga/commands-common.h b/qga/commands-common.h
new file mode 100644
index 0000000..90785ed
--- /dev/null
+++ b/qga/commands-common.h
@@ -0,0 +1,21 @@
+/*
+ * QEMU Guest Agent common/cross-platform common commands
+ *
+ * Copyright (c) 2020 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef QGA_COMMANDS_COMMON_H
+#define QGA_COMMANDS_COMMON_H
+
+#include "qga-qapi-types.h"
+
+typedef struct GuestFileHandle GuestFileHandle;
+
+GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp);
+
+GuestFileRead *guest_file_read_unsafe(GuestFileHandle *gfh,
+ int64_t count, Error **errp);
+
+#endif
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index cc69b82..a52af03 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -26,6 +26,7 @@
#include "qemu/sockets.h"
#include "qemu/base64.h"
#include "qemu/cutils.h"
+#include "commands-common.h"
#ifdef HAVE_UTMPX
#include <utmpx.h>
@@ -237,12 +238,12 @@
RW_STATE_WRITING,
} RwState;
-typedef struct GuestFileHandle {
+struct GuestFileHandle {
uint64_t id;
FILE *fh;
RwState state;
QTAILQ_ENTRY(GuestFileHandle) next;
-} GuestFileHandle;
+};
static struct {
QTAILQ_HEAD(, GuestFileHandle) filehandles;
@@ -268,7 +269,7 @@
return handle;
}
-static GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp)
+GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp)
{
GuestFileHandle *gfh;
@@ -460,29 +461,14 @@
g_free(gfh);
}
-struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
- int64_t count, Error **errp)
+GuestFileRead *guest_file_read_unsafe(GuestFileHandle *gfh,
+ int64_t count, Error **errp)
{
- GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
GuestFileRead *read_data = NULL;
guchar *buf;
- FILE *fh;
+ FILE *fh = gfh->fh;
size_t read_count;
- if (!gfh) {
- return NULL;
- }
-
- if (!has_count) {
- count = QGA_READ_COUNT_DEFAULT;
- } else if (count < 0 || count >= UINT32_MAX) {
- error_setg(errp, "value '%" PRId64 "' is invalid for argument count",
- count);
- return NULL;
- }
-
- fh = gfh->fh;
-
/* explicitly flush when switching from writing to reading */
if (gfh->state == RW_STATE_WRITING) {
int ret = fflush(fh);
@@ -497,7 +483,6 @@
read_count = fread(buf, 1, count, fh);
if (ferror(fh)) {
error_setg_errno(errp, errno, "failed to read file");
- slog("guest-file-read failed, handle: %" PRId64, handle);
} else {
buf[read_count] = 0;
read_data = g_new0(GuestFileRead, 1);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index b49920e..9717a8d 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -37,6 +37,7 @@
#include "qemu/queue.h"
#include "qemu/host-utils.h"
#include "qemu/base64.h"
+#include "commands-common.h"
#ifndef SHTDN_REASON_FLAG_PLANNED
#define SHTDN_REASON_FLAG_PLANNED 0x80000000
@@ -50,11 +51,11 @@
#define INVALID_SET_FILE_POINTER ((DWORD)-1)
-typedef struct GuestFileHandle {
+struct GuestFileHandle {
int64_t id;
HANDLE fh;
QTAILQ_ENTRY(GuestFileHandle) next;
-} GuestFileHandle;
+};
static struct {
QTAILQ_HEAD(, GuestFileHandle) filehandles;
@@ -126,7 +127,7 @@
return handle;
}
-static GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp)
+GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp)
{
GuestFileHandle *gfh;
QTAILQ_FOREACH(gfh, &guest_file_state.filehandles, next) {
@@ -321,39 +322,19 @@
}
}
-GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
- int64_t count, Error **errp)
+GuestFileRead *guest_file_read_unsafe(GuestFileHandle *gfh,
+ int64_t count, Error **errp)
{
GuestFileRead *read_data = NULL;
guchar *buf;
- HANDLE fh;
+ HANDLE fh = gfh->fh;
bool is_ok;
DWORD read_count;
- GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
- if (!gfh) {
- return NULL;
- }
- if (!has_count) {
- count = QGA_READ_COUNT_DEFAULT;
- } else if (count < 0 || count >= UINT32_MAX) {
- error_setg(errp, "value '%" PRId64
- "' is invalid for argument count", count);
- return NULL;
- }
-
- fh = gfh->fh;
- buf = g_try_malloc0(count + 1);
- if (!buf) {
- error_setg(errp,
- "failed to allocate sufficient memory "
- "to complete the requested service");
- return NULL;
- }
+ buf = g_malloc0(count + 1);
is_ok = ReadFile(fh, buf, count, &read_count, NULL);
if (!is_ok) {
error_setg_win32(errp, GetLastError(), "failed to read file");
- slog("guest-file-read failed, handle %" PRId64, handle);
} else {
buf[read_count] = 0;
read_data = g_new0(GuestFileRead, 1);
diff --git a/qga/commands.c b/qga/commands.c
index 4471a9f..efc8b90 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -11,6 +11,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "guest-agent-core.h"
#include "qga-qapi-commands.h"
#include "qapi/error.h"
@@ -18,11 +19,18 @@
#include "qemu/base64.h"
#include "qemu/cutils.h"
#include "qemu/atomic.h"
+#include "commands-common.h"
/* Maximum captured guest-exec out_data/err_data - 16MB */
#define GUEST_EXEC_MAX_OUTPUT (16*1024*1024)
/* Allocation and I/O buffer for reading guest-exec out_data/err_data - 4KB */
#define GUEST_EXEC_IO_SIZE (4*1024)
+/*
+ * Maximum file size to read - 48MB
+ *
+ * (48MB + Base64 3:4 overhead = JSON parser 64 MB limit)
+ */
+#define GUEST_FILE_READ_COUNT_MAX (48 * MiB)
/* Note: in some situations, like with the fsfreeze, logging may be
* temporarilly disabled. if it is necessary that a command be able
@@ -547,3 +555,28 @@
g_free(info);
return NULL;
}
+
+GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
+ int64_t count, Error **errp)
+{
+ GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
+ GuestFileRead *read_data;
+
+ if (!gfh) {
+ return NULL;
+ }
+ if (!has_count) {
+ count = QGA_READ_COUNT_DEFAULT;
+ } else if (count < 0 || count > GUEST_FILE_READ_COUNT_MAX) {
+ error_setg(errp, "value '%" PRId64 "' is invalid for argument count",
+ count);
+ return NULL;
+ }
+
+ read_data = guest_file_read_unsafe(gfh, count, errp);
+ if (!read_data) {
+ slog("guest-file-write failed, handle: %" PRId64, handle);
+ }
+
+ return read_data;
+}
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index f6fcb59..4be9aad 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -266,11 +266,13 @@
##
# @guest-file-read:
#
-# Read from an open file in the guest. Data will be base64-encoded
+# Read from an open file in the guest. Data will be base64-encoded.
+# As this command is just for limited, ad-hoc debugging, such as log
+# file access, the number of bytes to read is limited to 48 MB.
#
# @handle: filehandle returned by guest-file-open
#
-# @count: maximum number of bytes to read (default is 4KB)
+# @count: maximum number of bytes to read (default is 4KB, maximum is 48MB)
#
# Returns: @GuestFileRead on success.
#
