qemu-ga patch queue for hard-freeze

* enforce 48MB limit for guest-file-read to avoid memory allocation
   failures
 -----BEGIN PGP SIGNATURE-----
 
 iQFOBAABCgA4FiEEzqzJ4VU066u4LT+gM1PJzvEItYQFAl6XGPQaHG1kcm90aEBs
 aW51eC52bmV0LmlibS5jb20ACgkQM1PJzvEItYQBsgf/Y8C/ZtV0QTYDQLhZV9o2
 GrSlZrWsub1XLwL0ykKNAm1AYaimv08KebyV53fUz0IvRiw9Y9x+FEHFXLOflbGA
 EB7Jut8pOSo9MEPpmkyEkF4eFA5ur4xww+mJXbnTrgEJfRwnuRrmrpznzH09iiN/
 sgCGDJLb/Oqr2hmTn/xShNPyWT7bk1QAr2wUKEPlsQK4x+BVir3HRT+ZxTP09uJ/
 Rf2L2S/3CqJC7Y/No+JEeVs7BvyAop1jzlm5E2jrQV3M7/tmZITjSPDbE3nkqwUH
 RGTwCv0pbm/b9FAiJP8MZjNWn43dD4uA57fG9ixyjUs2b5ZGpLVbBGEHcjNWpBNr
 vA==
 =+IAh
 -----END PGP SIGNATURE-----

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>
This commit is contained in:
Peter Maydell 2020-04-15 17:03:50 +01:00
commit 6329df5b53
5 changed files with 73 additions and 51 deletions

21
qga/commands-common.h Normal file
View File

@ -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

View File

@ -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 @@ typedef enum {
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 @@ static int64_t guest_file_handle_add(FILE *fh, Error **errp)
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 @@ void qmp_guest_file_close(int64_t handle, Error **errp)
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 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
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);

View File

@ -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 @@ static int64_t guest_file_handle_add(HANDLE fh, Error **errp)
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 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
}
}
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);

View File

@ -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 @@ error:
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;
}

View File

@ -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.
#