Commit Graph
46 Commits
Author SHA1 Message Date
Carlos LópezandMichael S. Tsirkin 9c1916057a libvhost-user: check for NULL when allocating a virtqueue element
Check the return value for malloc(), avoiding a NULL pointer
dereference, and propagate error in function callers.

Found with GCC 13 and -fanalyzer:

../subprojects/libvhost-user/libvhost-user.c: In function ‘virtqueue_alloc_element’:
../subprojects/libvhost-user/libvhost-user.c:2556:19: error: dereference of possibly-NULL ‘elem’ [CWE-690] [-Werror=analyzer-possible-null-dereference]
 2556 |     elem->out_num = out_num;
      |     ~~~~~~~~~~~~~~^~~~~~~~~
  ‘virtqueue_alloc_element’: event 1
    |
    | 2554 |     assert(sz >= sizeof(VuVirtqElement));
    |      |     ^~~~~~
    |      |     |
    |      |     (1) following ‘true’ branch (when ‘sz > 31’)...
    |
  ‘virtqueue_alloc_element’: events 2-4
    |
    | 2555 |     elem = malloc(out_sg_end);
    |      |     ^~~~   ~~~~~~~~~~~~~~~~~~
    |      |     |      |
    |      |     |      (3) this call could return NULL
    |      |     (2) ...to here
    | 2556 |     elem->out_num = out_num;
    |      |     ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                   |
    |      |                   (4) ‘elem’ could be NULL: unchecked value from (3)
    |

Signed-off-by: Carlos López <clopez@suse.de>
Message-Id: <20230210112514.16858-1-clopez@suse.de>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2023-03-02 19:13:51 -05:00
Maxime CoquelinandMichael S. Tsirkin e608feed51 libvhost-user: Adopt new backend naming
The Vhost-user specification changed feature and request
naming from _SLAVE_ to _BACKEND_.

This patch adopts the new naming convention.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Message-Id: <20230208203259.381326-3-maxime.coquelin@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2023-03-02 03:10:47 -05:00
Marcel HoltmannandPaolo Bonzini 8d5666d76b libvduse: Add extra compiler warnings
In case libvhost-user is used externally, that projects compiler
warnings might be more strict. Enforce an extra set of compiler warnings
to catch issues early on.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <08daa1896ad8824e17d57d6a970bc0b4bee73ece.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:35 +01:00
Marcel HoltmannandPaolo Bonzini 722b62d97d libvhost-user: Add extra compiler warnings
In case libvhost-user is used externally, that projects compiler
warnings might be more strict. Enforce an extra set of compiler warnings
to catch issues early on.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <737ebf2e697f8640558e6f73d96a692711f548f6.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:35 +01:00
Marcel HoltmannandPaolo Bonzini 950a2f2eff libvhost-user: Fix assignment in vring_set_avail_event
Since it was proposed to change the code in libvduse.c to use memcpy
instead of an assignment, the code in libvhost-user.c should also be
changed to use memcpy.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <502b22723264db064e4b05008233a9c1f2f8aaaa.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:35 +01:00
Marcel HoltmannandPaolo Bonzini 86e61e4233 libvduse: Fix assignment in vring_set_avail_event
Since the assignment is causing a compiler warning, fix it by using
memcpy instead.

  CC       libvduse.o
libvduse.c: In function ‘vring_set_avail_event’:
libvduse.c:603:7: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasin]
  603 |     *((uint16_t *)&vq->vring.used->ring[vq->vring.num]) = htole16(val);
      |      ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Suggested-by: Xie Yongji <xieyongji@bytedance.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <4a0fe2a6436464473119fdbf0bc4076b36fbb37f.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:35 +01:00
Marcel HoltmannandPaolo Bonzini 85899f8e6b libvduse: Switch to unsigned int for inuse field in struct VduseVirtq
It seems there is no need to keep the inuse field signed and end up with
compiler warnings for sign-compare.

  CC       libvduse.o
libvduse.c: In function ‘vduse_queue_pop’:
libvduse.c:789:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
  789 |     if (vq->inuse >= vq->vring.num) {
      |                   ^~

Instead of casting the comparison to unsigned int, just make the inuse
field unsigned int in the fist place.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Reviewed-by: Xie Yongji <xieyongji@bytedance.com>
Message-Id: <9fe3fd8b042e048bd04d506ca6e43d738b5c45b7.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:34 +01:00
Marcel HoltmannandPaolo Bonzini 518ac42879 libvduse: Provide _GNU_SOURCE when compiling outside of QEMU
When the libvduse sources are used by another project, it can not be
guaranteed that _GNU_SOURCE is set by the build system. If it is for
example not set, errors like this show up.

  CC       libvduse.o
libvduse.c: In function ‘vduse_log_get’:
libvduse.c:172:9: error: implicit declaration of function ‘ftruncate’; did you mean ‘strncat’? [-Werror=implicit-function-declaration]
  172 |     if (ftruncate(fd, size) == -1) {
      |         ^~~~~~~~~
      |         strncat

The simplest way to allow external complication of libvduse.[ch] by
setting _GNU_SOURCE if it is not already set by the build system.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Message-Id: <407f3665f0605df936e5bfe60831d180edfb8cca.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:34 +01:00
Marcel HoltmannandPaolo Bonzini f1c563d209 libvhost-user: Change dev->postcopy_ufd assignment to make it C90 compliant
The assignment of dev->postcopy_ufd can be moved into an else clause and
then the code becomes C90 compliant.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_set_postcopy_advise’:
libvhost-user.c:1625:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 1625 |     struct uffdio_api api_struct;
      |     ^~~~~~

Understandable, it might be desired to avoid else clauses, but in this
case it seems clear enough and frankly the dev->postcopy_ufd is only
assigned once.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <74db52afb1203c4580ffc7fa462b4b2ba260a353.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:34 +01:00
Marcel HoltmannandPaolo Bonzini d87a642403 libvhost-user: Declare uffdio_register early to make it C90 compliant
When using libvhost-user source in an external project that wants to
comply with the C90 standard, it is best to declare variables before
code.

  CC       libvhost-user.o
libvhost-user.c: In function ‘generate_faults’:
libvhost-user.c:683:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
  683 |         struct uffdio_register reg_struct;
      |         ^~~~~~

In this case, it is also simple enough and doesn't cause any extra
ifdef additions.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <556c2d00c01fa134d13c0371d4014c90694c2943.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:34 +01:00
Marcel HoltmannandPaolo Bonzini 92bf246130 libvhost-user: Use unsigned int i for some for-loop iterations
The sign-compare warning also hits some of the for-loops, but it easy
fixed by just making the iterator variable unsigned int.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_gpa_to_va’:
libvhost-user.c:223:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare]
  223 |     for (i = 0; i < dev->nregions; i++) {
      |                   ^

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Message-Id: <decb925e1a6fb9538738d2570bda2804f888fa15.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:34 +01:00
Marcel HoltmannandPaolo Bonzini 18fa7f1e95 libvhost-user: Cast rc variable to avoid compiler warning
The assert from recvmsg() return value against an uint32_t size field
from a protocol struct throws a compiler warning.

  CC       libvhost-user.o
In file included from libvhost-user.c:27:
libvhost-user.c: In function ‘vu_message_read_default’:
libvhost-user.c:363:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare]
  363 |         assert(rc == vmsg->size);
      |                   ^~

This is not critical, but annoying when the libvhost-user source are
used in an external project that has this compiler warning switched on.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Message-Id: <7a791e27b7bd3e0a8b8cc8fbb15090a870d226d5.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:34 +01:00
Marcel HoltmannandPaolo Bonzini aa5d395ac4 libvhost-user: Replace typeof with __typeof__
Strictly speaking only -std=gnu99 support the usage of typeof and for
easier inclusion in external projects, it is better to use __typeof__.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_log_queue_fill’:
libvhost-user.c:86:13: error: implicit declaration of function ‘typeof’ [-Werror=implicit-function-declaration]
   86 |             typeof(x) _min1 = (x);              \
      |             ^~~~~~

Changing these two users of typeof makes the compiler happy and no extra
flags or pragmas need to be provided.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <981aa822bcaaa2b8d74f245339a99a85c25b346f.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:34 +01:00
Marcel HoltmannandPaolo Bonzini dadc3d01bc libvhost-user: Provide _GNU_SOURCE when compiling outside of QEMU
Then the libvhost-user sources are used by another project, it can not
be guaranteed that _GNU_SOURCE is set by the build system. If it is for
example not set, errors like this show up.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_panic’:
libvhost-user.c:195:9: error: implicit declaration of function ‘vasprintf’; did you mean ‘vsprintf’? [-Werror=implicit-function-declaration]
  195 |     if (vasprintf(&buf, msg, ap) < 0) {
      |         ^~~~~~~~~
      |         vsprintf

The simplest way to allow external complication of libvhost-user.[ch] is
by setting _GNU_SOURCE if it is not already set by the build system.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Message-Id: <be27dcc747a6b5cc6f8ae3f79e0b79171382bcef.1671741278.git.marcel@holtmann.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2023-01-11 10:44:34 +01:00
Marcel HoltmannandMichael S. Tsirkin e7ee4fe24d libvhost-user: Switch to unsigned int for inuse field in struct VuVirtq
It seems there is no need to keep the inuse field signed and end up with
compiler warnings for sign-compare.

  CC       libvhost-user.o
libvhost-user.c: In function ‘vu_queue_pop’:
libvhost-user.c:2763:19: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
 2763 |     if (vq->inuse >= vq->vring.num) {
      |                   ^~
libvhost-user.c: In function ‘vu_queue_rewind’:
libvhost-user.c:2808:13: error: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Werror=sign-compare]
 2808 |     if (num > vq->inuse) {
      |             ^

Instead of casting the comparision to unsigned int, just make the inuse
field unsigned int in the fist place.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Message-Id: <20221219175337.377435-8-marcel@holtmann.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2022-12-21 07:32:24 -05:00
Stefan Weil viaandStefan Hajnoczi 52a57d8d1f libvhost-user: Add format attribute to local function vu_panic
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220422070144.1043697-4-sw@weilnetz.de>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20221126152507.283271-5-sw@weilnetz.de>
2022-11-27 13:33:58 -05:00
Stefan Weil viaandStefan Hajnoczi 7d4774e681 libvhost-user: Fix two more format strings
This fix is required for 32 bit hosts. The bug was detected by CI
for arm-linux, but is also relevant for i386-linux.

Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20221126152507.283271-4-sw@weilnetz.de>
2022-11-27 13:33:58 -05:00
Stefan Weil viaandStefan Hajnoczi 8541bf452d libvhost-user: Fix format strings
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220422070144.1043697-3-sw@weilnetz.de>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20221126152507.283271-3-sw@weilnetz.de>
2022-11-27 13:33:58 -05:00
Stefan Weil viaandStefan Hajnoczi 266aaedc37 libvhost-user: Fix wrong type of argument to formatting function (reported by LGTM)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Message-Id: <20220422070144.1043697-2-sw@weilnetz.de>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20221126152507.283271-2-sw@weilnetz.de>
2022-11-27 13:33:57 -05:00
Philippe Mathieu-DaudéandStefan Hajnoczi 57bc6e40e8 libvduse: Avoid warning about dangerous use of strncpy()
GCC 8 added a -Wstringop-truncation warning:

  The -Wstringop-truncation warning added in GCC 8.0 via r254630 for
  bug 81117 is specifically intended to highlight likely unintended
  uses of the strncpy function that truncate the terminating NUL
  character from the source string.

Here the next line indeed unconditionally zeroes the last byte, but
1/ the buffer has been calloc'd, so we don't need to add an extra
byte, and 2/ we called vduse_name_is_invalid() which checked the
string length, so we can simply call strcpy().

This fixes when using gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0:

  [42/666] Compiling C object subprojects/libvduse/libvduse.a.p/libvduse.c.o
  FAILED: subprojects/libvduse/libvduse.a.p/libvduse.c.o
  cc -m64 -mcx16 -Isubprojects/libvduse/libvduse.a.p -Isubprojects/libvduse -I../../subprojects/libvduse [...] -o subprojects/libvduse/libvduse.a.p/libvduse.c.o -c ../../subprojects/libvduse/libvduse.c
  In file included from /usr/include/string.h:495,
                   from ../../subprojects/libvduse/libvduse.c:24:
  In function ‘strncpy’,
      inlined from ‘vduse_dev_create’ at ../../subprojects/libvduse/libvduse.c:1312:5:
  /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ specified bound 256 equals destination size [-Werror=stringop-truncation]
    106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors
  ninja: build stopped: cannot make progress due to previous errors.

Fixes: d9cf16c0be ("libvduse: Replace strcpy() with strncpy()")
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Xie Yongji <xieyongji@bytedance.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Bin Meng <bmeng@tinylab.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20221111124550.35753-1-philmd@linaro.org>
2022-11-12 09:15:59 -05:00
Xie YongjiandKevin Wolf 630179b7f7 libvduse: Pass positive value to strerror()
The value passed to strerror() should be positive.
So let's fix it.

Fixes: Coverity CID 1490226, 1490223
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20220706095624.328-4-xieyongji@bytedance.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-08-02 11:01:42 +02:00
Xie YongjiandKevin Wolf d9cf16c0be libvduse: Replace strcpy() with strncpy()
Coverity reported a string overflow issue since we copied
"name" to "dev_config->name" without checking the length.
This should be a false positive since we already checked
the length of "name" in vduse_name_is_invalid(). But anyway,
let's replace strcpy() with strncpy() (as a general library,
we'd like to minimize dependencies on other libraries, so we
didn't use g_strlcpy() here) to fix the coverity complaint.

Fixes: Coverity CID 1490224
Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20220706095624.328-3-xieyongji@bytedance.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-08-02 11:01:41 +02:00
Xie YongjiandKevin Wolf e7156ff7cb libvduse: Fix the incorrect function name
In vduse_name_is_valid(), we actually check whether
the name is invalid or not. So let's change the
function name to vduse_name_is_invalid() to match
the behavior.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20220706095624.328-2-xieyongji@bytedance.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2022-08-02 11:01:41 +02:00
Kevin WolfandMichael S. Tsirkin 7f27d20ded libvhost-user: Fix VHOST_USER_ADD_MEM_REG reply
With REPLY_NEEDED, libvhost-user sends both the acutal result and an
additional ACK reply for VHOST_USER_ADD_MEM_REG. This is incorrect, the
spec mandates that it behave the same with and without REPLY_NEEDED
because it always sends a reply.

Fixes: ec94c8e621
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20220627134500.94842-4-kwolf@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2022-06-27 18:53:18 -04:00
Kevin WolfandMichael S. Tsirkin 69a5daec06 libvhost-user: Fix VHOST_USER_GET_MAX_MEM_SLOTS reply
With REPLY_NEEDED, libvhost-user sends both the acutal result and an
additional ACK reply for VHOST_USER_GET_MAX_MEM_SLOTS. This is
incorrect, the spec mandates that it behave the same with and without
REPLY_NEEDED because it always sends a reply.

Fixes: 6fb2e173d2
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20220627134500.94842-3-kwolf@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2022-06-27 18:53:18 -04:00