[Qemu-commits] [qemu/qemu] f62ebb: Revert "prevent crash when executing guest-file-re...

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: f62ebb63848107336e57adc12369aefaa639e38c
      
https://github.com/qemu/qemu/commit/f62ebb63848107336e57adc12369aefaa639e38c
  Author: Philippe Mathieu-Daudé <address@hidden>
  Date:   2020-04-15 (Wed, 15 Apr 2020)

  Changed paths:
    M qga/commands-win32.c

  Log Message:
  -----------
  Revert "prevent crash when executing guest-file-read with large count"

As noted by Daniel Berrangé in [*], the fix from commit 807e2b6fce
which replaced malloc() by try_malloc() is not enough, the process
can still run out of memory a few line later:

 346     buf = g_try_malloc0(count + 1);
 347     if (!buf) {
 348         error_setg(errp,
 349                    "failed to allocate sufficient memory "
 350                    "to complete the requested service");
 351         return NULL;
 352     }
 353     is_ok = ReadFile(fh, buf, count, &read_count, NULL);
 354     if (!is_ok) {
 355         error_setg_win32(errp, GetLastError(), "failed to read file");
 356         slog("guest-file-read failed, handle %" PRId64, handle);
 357     } else {
 358         buf[read_count] = 0;
 359         read_data = g_new0(GuestFileRead, 1);
                         ^^^^^^

Instead we are going to put a low hard limit on 'count' in the next
commits. This reverts commit 807e2b6fce022707418bc8f61c069d91c613b3d2.

[*] https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg03471.html

Suggested-by: Daniel P. Berrangé <address@hidden>
Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
Signed-off-by: Michael Roth <address@hidden>


  Commit: 5d3586b834633c8ac462d4741b85b4036cbc0f93
      
https://github.com/qemu/qemu/commit/5d3586b834633c8ac462d4741b85b4036cbc0f93
  Author: Philippe Mathieu-Daudé <address@hidden>
  Date:   2020-04-15 (Wed, 15 Apr 2020)

  Changed paths:
    A qga/commands-common.h
    M qga/commands-posix.c
    M qga/commands-win32.c

  Log Message:
  -----------
  qga: Extract guest_file_handle_find() to commands-common.h

As we are going to reuse this method, declare it in common
header.

Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
Signed-off-by: Michael Roth <address@hidden>


  Commit: ead83a136d54f7faa315922aff26fa11d216909f
      
https://github.com/qemu/qemu/commit/ead83a136d54f7faa315922aff26fa11d216909f
  Author: Philippe Mathieu-Daudé <address@hidden>
  Date:   2020-04-15 (Wed, 15 Apr 2020)

  Changed paths:
    M qga/commands-common.h
    M qga/commands-posix.c
    M qga/commands-win32.c
    M qga/commands.c

  Log Message:
  -----------
  qga: Extract qmp_guest_file_read() to common commands.c

Extract the common code shared by both POSIX/Win32 implementations.

Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
Signed-off-by: Michael Roth <address@hidden>


  Commit: 1329651fb4d4c5068ad12fd86aff7e52f9e18c34
      
https://github.com/qemu/qemu/commit/1329651fb4d4c5068ad12fd86aff7e52f9e18c34
  Author: Philippe Mathieu-Daudé <address@hidden>
  Date:   2020-04-15 (Wed, 15 Apr 2020)

  Changed paths:
    M qga/commands.c
    M qga/qapi-schema.json

  Log Message:
  -----------
  qga: Restrict guest-file-read count to 48 MB to avoid crashes

On [*] Daniel Berrangé commented:

  The QEMU guest agent protocol is not sensible way to access huge
  files inside the guest. It requires the inefficient process of
  reading the entire data into memory than duplicating it again in
  base64 format, and then copying it again in the JSON serializer /
  monitor code.

  For arbitrary general purpose file access, especially for large
  files, use a real file transfer program or use a network block
  device, not the QEMU guest agent.

To avoid bug reports as BZ#1594054 (CVE-2018-12617), follow his
suggestion to put a low, hard limit on "count" in the guest agent
QAPI schema, and don't allow count to be larger than 48 MB.

[*] https://www.mail-archive.com/address@hidden/msg693176.html

Fixes: CVE-2018-12617
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1594054
Reported-by: Fakhri Zulkifli <address@hidden>
Suggested-by: Daniel P. Berrangé <address@hidden>
Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
Reviewed-by: Daniel P. Berrangé <address@hidden>
*update schema documentation to indicate 48MB limit instead of 10MB
Signed-off-by: Michael Roth <address@hidden>


  Commit: 6329df5b53a3307f289451137c9910da0f09adc6
      
https://github.com/qemu/qemu/commit/6329df5b53a3307f289451137c9910da0f09adc6
  Author: Peter Maydell <address@hidden>
  Date:   2020-04-15 (Wed, 15 Apr 2020)

  Changed paths:
    A qga/commands-common.h
    M qga/commands-posix.c
    M qga/commands-win32.c
    M qga/commands.c
    M qga/qapi-schema.json

  Log Message:
  -----------
  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 "address@hidden"
# gpg: Good signature from "Michael Roth <address@hidden>" [full]
# gpg:                 aka "Michael Roth <address@hidden>" [full]
# gpg:                 aka "Michael Roth <address@hidden>" [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 <address@hidden>


Compare: https://github.com/qemu/qemu/compare/73995d15557a...6329df5b53a3