Commit Graph

39 Commits

Author SHA1 Message Date
Fam Zheng 6d4b9e55fc curl: check data size before memcpy to local buffer. (CVE-2014-0144)
curl_read_cb is callback function for libcurl when data arrives. The
data size passed in here is not guaranteed to be within the range of
request we submitted, so we may overflow the guest IO buffer. Check the
real size we have before memcpy to buffer to avoid overflow.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2014-04-01 14:19:09 +02:00
Paolo Bonzini 2a94fee3f6 curl: correctly propagate errors
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-02-21 21:02:23 +01:00
Markus Armbruster 84d18f065f Use error_is_set() only when necessary
error_is_set(&var) is the same as var != NULL, but it takes
whole-program analysis to figure that out.  Unnecessarily hard for
optimizers, static checkers, and human readers.  Dumb it down to
obvious.

Gets rid of several dozen Coverity false positives.

Note that the obvious form is already used in many places.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2014-02-17 11:57:23 -05:00
Peter Maydell 031fd1be56 block/curl: Implement the libcurl timer callback interface
libcurl versions 7.16.0 and later have a timer callback interface which
must be implemented in order for libcurl to make forward progress (it
will sometimes rely on being called back on the timeout if there are
no file descriptors registered). Implement the callback, and use a
QEMU AIO timer to ensure we prod libcurl again when it asks us to.

Based on Peter's original patch plus my fix to add curl_multi_timeout_do.
Should compile just fine even on older versions of libcurl.

I also tried copy-on-read and streaming:

    $ ./qemu-img create -f qcow2 -o \
         backing_file=http://download.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso \
         foo.qcow2 1G
    $ x86_64-softmmu/qemu-system-x86_64 \
         -drive if=none,file=foo.qcow2,copy-on-read=on,id=cd \
         -device ide-cd,drive=cd --enable-kvm -m 1024

Direct http usage is probably too slow, but with copy-on-read ultimately
the image does boot!

After some time, streaming gets canceled by an EIO, which needs further
investigation.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2014-01-24 16:07:08 +01:00
Peter Crosthwaite 87ea75d5e1 qemu-option: Remove qemu_opts_create_nofail
This is a boiler-plate _nofail variant of qemu_opts_create. Remove and
use error_abort in call sites.

null/0 arguments needs to be added for the id and fail_if_exists fields
in affected callsites due to argument inconsistency between the normal and
no_fail variants.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
2014-01-06 15:02:30 -05:00
Max Reitz 015a1036a7 bdrv: Use "Error" for opening images
Add an Error ** parameter to BlockDriver.bdrv_open and
BlockDriver.bdrv_file_open to allow more specific error messages.

Signed-off-by: Max Reitz <mreitz@redhat.com>
2013-09-12 10:12:47 +02:00
Stefan Hajnoczi 5b21a2ae4d curl: qemu_bh_new() can never return NULL
Drop error code path which cannot be taken since qemu_bh_new() does not
return NULL.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2013-09-01 19:11:56 +04:00
Stefan Hajnoczi f2e5dca46b aio: drop io_flush argument
The .io_flush() handler no longer exists and has no users.  Drop the
io_flush argument to aio_set_fd_handler() and related functions.

The AioFlushEventNotifierHandler and AioFlushHandler typedefs are no
longer used and are dropped too.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2013-08-19 15:52:19 +02:00
Stefan Hajnoczi 0d1460226f block/curl: drop curl_aio_flush()
.io_flush() is no longer called so drop curl_aio_flush().  The acb[]
array that the function checks is still used in other parts of
block/curl.c.  Therefore we cannot remove acb[], it is needed.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2013-08-19 15:45:35 +02:00
Fam Zheng 3494d65027 curl: refuse to open URL from HTTP server without range support
CURL driver requests partial data from server on guest IO req. For HTTP
and HTTPS, it uses "Range: ***" in requests, and this will not work if
server not accepting range. This patch does this check when open.

 * Removed curl_size_cb, which is not used: On one hand it's registered to
   libcurl as CURLOPT_WRITEFUNCTION, instead of CURLOPT_HEADERFUNCTION,
   which will get called with *data*, not *header*. On the other hand the
   s->len is assigned unconditionally later.

   In this gone function, the sscanf for "Content-Length: %zd", on
   (void *)ptr, which is not guaranteed to be zero-terminated, is
   potentially a security bug. So this patch fixes it as a side-effect. The
   bug is reported as: https://bugs.launchpad.net/qemu/+bug/1188943
   (Note the bug is marked "private" so you might not be able to see it)

 * Introduced curl_header_cb, which is used to parse header and mark the
   server as accepting range if "Accept-Ranges: bytes" line is seen from
   response header. If protocol is HTTP or HTTPS, but server response has
   no not this support, refuse to open this URL.

Note that python builtin module SimpleHTTPServer is an example of not
supporting range, if you need to test this driver, get a better server
or use internet URLs.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2013-07-05 09:40:18 +02:00
Richard W.M. Jones 8da1aa15db curl: Don't set curl options on the handle just before it's going to be deleted.
(Found by Kamil Dudka)

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-06-17 17:47:59 +02:00
Richard W.M. Jones a7cea2ba47 block/curl.c: Refuse to open the handle for writes.
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2013-06-17 17:47:59 +02:00
Richard W.M. Jones 9e5e2b23d3 curl: Whitespace only changes.
Trivial patch to remove odd whitespace.

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2013-06-11 23:45:43 +04:00
Kevin Wolf 56d1b4d21d block: Remove filename parameter from .bdrv_file_open()
It is unused now in all block drivers.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2013-04-22 11:34:35 +02:00
Kevin Wolf 8e6d58cd5b curl: Use bdrv_open options instead of filename
As a bonus, going through the QemuOpts QEMU_OPT_SIZE parser for the
readahead option gives us proper error reporting that the previous use
of atoi() lacked.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2013-04-22 10:27:59 +02:00
Kevin Wolf 787e4a8500 block: Add options QDict to bdrv_file_open() prototypes
The new parameter is unused yet.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2013-03-22 17:51:31 +01:00
Stefan Hajnoczi 8a8f584008 block/curl: only restrict protocols with libcurl>=7.19.4
The curl_easy_setopt(state->curl, CURLOPT_PROTOCOLS, ...) interface was
introduced in libcurl 7.19.4.  Therefore we cannot protect against
CVE-2013-0249 when linking against an older libcurl.

This fixes the build failure introduced by
fb6d1bbd24.

Reported-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Andreas Färber <andreas.faeber@web.de>
Message-id: 1360743934-8337-1-git-send-email-stefanha@redhat.com
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
2013-02-13 11:57:35 -06:00
Stefan Hajnoczi fb6d1bbd24 block/curl: disable extra protocols to prevent CVE-2013-0249
There is a buffer overflow in libcurl POP3/SMTP/IMAP.  The workaround is
simple: disable extra protocols so that they cannot be exploited.  Full
details here:

  http://curl.haxx.se/docs/adv_20130206.html

QEMU only cares about HTTP, HTTPS, FTP, FTPS, and TFTP.  I have tested
that this fix prevents the exploit on my host with
libcurl-7.27.0-5.fc18.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
2013-02-08 11:14:20 -06:00
Paolo Bonzini 737e150e89 block: move include files to include/block/
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2012-12-19 08:31:31 +01:00
Stefan Hajnoczi d7331bed11 aio: rename AIOPool to AIOCBInfo
Now that AIOPool no longer keeps a freelist, it isn't really a "pool"
anymore.  Rename it to AIOCBInfo and make it const since it no longer
needs to be modified.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2012-11-14 18:19:21 +01:00
Stefan Weil 45724d6d02 block/curl: Fix wrong free statement
Report from smatch:
block/curl.c:546 curl_close(21) info: redundant null check on s->url calling free()

The check was redundant, and free was also wrong because the memory
was allocated using g_strdup.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2012-09-12 15:50:09 +02:00
Michael Tokarev 03396148bc allow qemu_iovec_from_buffer() to specify offset from which to start copying
Similar to
 qemu_iovec_memset(QEMUIOVector *qiov, size_t offset,
                   int c, size_t bytes);
the new prototype is:
 qemu_iovec_from_buf(QEMUIOVector *qiov, size_t offset,
                     const void *buf, size_t bytes);

The processing starts at offset bytes within qiov.

This way, we may copy a bounce buffer directly to
a middle of qiov.

This is exactly the same function as iov_from_buf() from
iov.c, so use the existing implementation and rename it
to qemu_iovec_from_buf() to be shorter and to match the
utility function.

As with utility implementation, we now assert that the
offset is inside actual iovec.  Nothing changed for
current callers, because `offset' parameter is new.

While at it, stop using "bounce-qiov" in block/qcow2.c
and copy decrypted data directly from cluster_data
instead of recreating a temp qiov for doing that.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
2012-06-11 23:12:11 +04:00
Paolo Bonzini bafbd6a1c6 aio: remove process_queue callback and qemu_aio_process_queue
Both unused after the previous patch.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2012-04-19 16:37:53 +02:00
Stefan Weil fb7c8e8a2d block/curl: Replace usleep by g_usleep
The function usleep is not available for all supported platforms:
at least some versions of MinGW don't support it.

usleep was also declared obsolete by POSIX.1-2001.

The function g_usleep is part of glib2.0, so it is available for
all supported platforms.

Using nanosleep would also be possible but needs more code.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
2012-04-03 09:34:34 +01:00
Paolo Bonzini 91977c2e5f block: qemu_aio_get does not return NULL
Initially done with the following semantic patch:

@ rule1 @
expression E;
statement S;
@@
  E = qemu_aio_get (...);
(
- if (E == NULL) { ... }
|
- if (E)
    { <... S ...> }
)

which however missed occurrences in linux-aio.c and posix-aio-compat.c.
Those were done by hand.

The change in vdi_aio_setup's caller was also done by hand.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2011-12-15 12:40:08 +01:00
Nick Thomas 363c3c8535 block/curl: Don't finish AIOCBs too early
The previous behaviour was to finish AIOCBs inside curl_aio_readv()
if the data was cached. This caused the following failed assertion
at hw/ide/pci.c:314: bmdma_cmd_writeb

"Assertion `bm->bus->dma->aiocb == ((void *)0)' failed."

By scheduling a QEMUBH and performing the completion inside the
callback, we avoid this problem.

Signed-off-by: Nick Thomas <nick@bytemark.co.uk>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2011-09-21 15:16:04 +02:00
Nick Thomas c84dcdc1d6 block/curl: Implement a flush function on the fd handlers
Signed-off-by: Nick Thomas <nick@bytemark.co.uk>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2011-09-21 15:16:02 +02:00
Nicholas Thomas f785a5ae36 block/curl: Handle failed reads gracefully.
Current behaviour if a read fails is for the acb to not get finished.
This causes an infinite loop in bdrv_read_em (block.c). The read failure
never gets reported to the  guest and if the error condition clears, the
process never recovers.

With this patch, when curl reports a failure we finish the acb as a
failure. This results in the guest receiving an I/O error (rather than
the read hanging indefinitely) and if the error condition subsequently
clears, retries work as expected.

The simplest test is to put an ISO on a web server you have control over
and open it with qemu-io. Then move the ISO out of the way and attempt
to read some data - you should see behaviour matching the above.

Signed-off-by: Nick Thomas <nick@bytemark.co.uk>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2011-08-23 17:41:14 +02:00
Anthony Liguori 7267c0947d Use glib memory allocation and free functions
qemu_malloc/qemu_free no longer exist after this commit.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
2011-08-20 23:01:08 -05:00
Blue Swirl 0bfcd599e3 Fix %lld or %llx printf format use
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
2010-05-22 08:02:12 +00:00
Kevin Wolf 66f82ceed6 block: Open the underlying image file in generic code
Format drivers shouldn't need to bother with things like file names, but rather
just get an open BlockDriverState for the underlying protocol. This patch
introduces this behaviour for bdrv_open implementation. For protocols which
need to access the filename to open their file/device/connection/... a new
callback bdrv_file_open is introduced which doesn't get an underlying file
opened.

For now, also some of the more obscure formats use bdrv_file_open because they
open() the file themselves instead of using the block.c functions. They need to
be fixed in later patches.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2010-05-03 10:07:30 +02:00
Jim Meyering 6265eb26a3 don't dereference NULL after failed strdup
Most of these are obvious NULL-deref bug fixes, for example,
the ones in these files:

  block/curl.c
  net.c
  slirp/misc.c

and the first one in block/vvfat.c.
The others in block/vvfat.c may not lead to an immediate segfault, but I
traced the two schedule_rename(..., strdup(path)) uses, and a failed
strdup would appear to trigger this assertion in handle_renames_and_mkdirs:

	    assert(commit->path);

The conversion to use qemu_strdup in envlist_to_environ is not technically
needed, but does avoid a theoretical leak in the caller when strdup fails
for one value, but later succeeds in allocating another buffer(plausible,
if one string length is much larger than the others).  The caller does
not know the length of the returned list, and as such can only free
pointers until it hits the first NULL.  If there are non-NULL pointers
beyond the first, their buffers would be leaked.  This one is admittedly
far-fetched.

The two in linux-user/main.c are worth fixing to ensure that an
OOM error is diagnosed up front, rather than letting it provoke some
harder-to-diagnose secondary error, in case of exec failure, or worse, in
case the exec succeeds but with an invalid list of command line options.
However, considering how unlikely it is to encounter a failed strdup early
in main, this isn't a big deal.  Note that adding the required uses of
qemu_strdup here and in envlist.c induce link failures because qemu_strdup
is not currently in any library they're linked with.  So for now, I've
omitted those changes, as well as the fixes in target-i386/helper.c
and target-sparc/helper.c.

If you'd like to see the above discussion (or anything else)
in the commit log, just let me know and I'll be happy to adjust.

>From 9af42864fd1ea666bd25e2cecfdfae74c20aa8c7 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Mon, 8 Feb 2010 18:29:29 +0100
Subject: [PATCH] don't dereference NULL after failed strdup

Handle failing strdup by replacing each use with qemu_strdup,
so as not to dereference NULL or trigger a failing assertion.
* block/curl.c (curl_open): s/\bstrdup\b/qemu_strdup/
* block/vvfat.c (init_directories): Likewise.
(get_cluster_count_for_direntry, check_directory_consistency): Likewise.
* net.c (parse_host_src_port): Likewise.
* slirp/misc.c (fork_exec): Likewise.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
2010-02-10 14:00:53 -06:00
malc 48a402e693 block/curl: %Z is not a valid conversion specifier
Signed-off-by: malc <av1474@comtv.ru>
2010-02-08 12:44:55 +03:00
malc d0f2c4c602 Do not use dprintf
dprintf is already claimed by POSIX[1], and on at least one system
is implemented as a macro

[1] http://www.opengroup.org/onlinepubs/9699919799/functions/dprintf.html

Signed-off-by: malc <av1474@comtv.ru>
2010-02-07 02:03:50 +03:00
Kevin Wolf 8febfa2684 Add qemu_aio_process_queue()
We'll leave some AIO completions unhandled when we can't call the callback.
qemu_aio_process_queue() is used later to run any callbacks that are left and
can be run then.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
2009-10-27 12:28:59 -05:00
Blue Swirl df3cee1a3a Fix Sparse warning about "expression using sizeof on a function"
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
2009-08-01 10:13:44 +00:00
Nolan c76f4952bb Allow adjustment of http block device's readahead size, via a new
":readahead=###:" suffix.

Signed-off-by: Nolan Leake <nolan <at> sigbus.net>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
2009-07-09 16:06:40 -05:00
Christoph Hellwig c16b5a2ca0 fully split aio_pool from BlockDriver
Now that we have a separate aio pool structure we can remove those
aio pool details from BlockDriver.

Every driver supporting AIO now needs to declare a static AIOPool
with the aiocb size and the cancellation method.  This cleans up the
current code considerably and will make it cleaner and more obvious
to support two different aio implementations behind a single
BlockDriver.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
2009-05-27 09:46:03 -05:00
Alexander Graf 769ce76d0e Add HTTP protocol using curl v6
Currently Qemu can read from posix I/O and NBD. This patch adds a
third protocol to the game: HTTP.

In certain situations it can be useful to access HTTP data directly,
for example if you want to try out an http provided OS image, but
don't know if you want to download it yet.

Using this patch you can now try it on on the fly. Just use it like:

qemu -cdrom http://host/path/my.iso

Signed-off-by: Alexander Graf <agraf@suse.de>
2009-05-22 10:50:35 -05:00