Re: [PATCH 05/10] vhost-backend: avoid overflow on memslots_limit

2021-11-11 Thread Roman Kagan
On Thu, Nov 11, 2021 at 06:59:43PM +0100, Philippe Mathieu-Daudé wrote: > On 11/11/21 16:33, Roman Kagan wrote: > > Fix the (hypothetical) potential problem when the value parsed out of > > the vhost module parameter in sysfs overflows the return value from > > vhost_kernel_memslots_limit. > > >

Re: [PATCH 01/10] vhost-user-blk: reconnect on any error during realize

2021-11-11 Thread Roman Kagan
On Thu, Nov 11, 2021 at 06:52:30PM +0100, Kevin Wolf wrote: > Am 11.11.2021 um 16:33 hat Roman Kagan geschrieben: > > vhost-user-blk realize only attempts to reconnect if the previous > > connection attempt failed on "a problem with the connection and not an > > error related to the content (which

Re: [PATCH-for-6.2] hw/nvme/ctrl: Fix buffer overrun (CVE-2021-3947)

2021-11-11 Thread Klaus Jensen
On Nov 11 19:46, Philippe Mathieu-Daudé wrote: > On 11/11/21 19:08, Klaus Jensen wrote: > > On Nov 11 16:31, Philippe Mathieu-Daudé wrote: > >> Both 'buf_len' and 'off' arguments are under guest control. > >> Since nvme_c2h() doesn't check out of boundary access, the > >> caller must check for

Re: [PATCH 00/10] vhost: stick to -errno error return convention

2021-11-11 Thread Michael S. Tsirkin
On Thu, Nov 11, 2021 at 06:33:44PM +0300, Roman Kagan wrote: > Error propagation between the generic vhost code and the specific backends is > not quite consistent: some places follow "return -1 and set errno" convention, > while others assume "return negated errno". Furthermore, not enough care

Re: [PATCH-for-6.2] hw/nvme/ctrl: Fix buffer overrun (CVE-2021-3947)

2021-11-11 Thread Philippe Mathieu-Daudé
On 11/11/21 19:08, Klaus Jensen wrote: > On Nov 11 16:31, Philippe Mathieu-Daudé wrote: >> Both 'buf_len' and 'off' arguments are under guest control. >> Since nvme_c2h() doesn't check out of boundary access, the >> caller must check for eventual buffer overrun on 'trans_len'. >> >> Cc:

Re: [PATCH-for-7.0 0/2] hw/nvme/ctrl: Buffer types cleanups

2021-11-11 Thread Klaus Jensen
On Nov 11 16:45, Philippe Mathieu-Daudé wrote: > Some trivial notes I took while reviewing CVE-2021-3947: > https://lore.kernel.org/qemu-devel/2021153125.2258176-1-phi...@redhat.com/ > > Based-on: <2021153125.2258176-1-phi...@redhat.com> > > *** BLURB HERE *** > > Philippe Mathieu-Daudé

Re: [PATCH-for-6.2] hw/nvme/ctrl: Fix buffer overrun (CVE-2021-3947)

2021-11-11 Thread Klaus Jensen
On Nov 11 16:31, Philippe Mathieu-Daudé wrote: > Both 'buf_len' and 'off' arguments are under guest control. > Since nvme_c2h() doesn't check out of boundary access, the > caller must check for eventual buffer overrun on 'trans_len'. > > Cc: qemu-sta...@nongnu.org > Reported-by: Qiuhao Li >

Re: [PATCH 06/10] vhost-backend: stick to -errno error return convention

2021-11-11 Thread Philippe Mathieu-Daudé
On 11/11/21 16:33, Roman Kagan wrote: > Almost all VhostOps methods in kernel_ops follow the convention of > returning negated errno on error. > > Adjust the only one that doesn't. > > Signed-off-by: Roman Kagan > --- > hw/virtio/vhost-backend.c | 2 +- > 1 file changed, 1 insertion(+), 1

Re: [PATCH 05/10] vhost-backend: avoid overflow on memslots_limit

2021-11-11 Thread Philippe Mathieu-Daudé
On 11/11/21 16:33, Roman Kagan wrote: > Fix the (hypothetical) potential problem when the value parsed out of > the vhost module parameter in sysfs overflows the return value from > vhost_kernel_memslots_limit. > > Signed-off-by: Roman Kagan > --- > hw/virtio/vhost-backend.c | 2 +- > 1 file

Re: [PATCH v2 0/3] virtio: increase VIRTQUEUE_MAX_SIZE to 32k

2021-11-11 Thread Christian Schoenebeck
On Donnerstag, 11. November 2021 17:31:52 CET Stefan Hajnoczi wrote: > On Wed, Nov 10, 2021 at 04:53:33PM +0100, Christian Schoenebeck wrote: > > On Mittwoch, 10. November 2021 16:14:19 CET Stefan Hajnoczi wrote: > > > On Wed, Nov 10, 2021 at 02:14:43PM +0100, Christian Schoenebeck wrote: > > > >

Re: [PATCH 01/10] vhost-user-blk: reconnect on any error during realize

2021-11-11 Thread Kevin Wolf
Am 11.11.2021 um 16:33 hat Roman Kagan geschrieben: > vhost-user-blk realize only attempts to reconnect if the previous > connection attempt failed on "a problem with the connection and not an > error related to the content (which would fail again the same way in the > next attempt)". > > However

Re: [PATCH v2 00/10] block: Attempt on fixing 030-reported errors

2021-11-11 Thread Kevin Wolf
Am 11.11.2021 um 13:08 hat Hanna Reitz geschrieben: > Hi, > > v1 cover letter: > https://lists.nongnu.org/archive/html/qemu-devel/2021-11/msg01287.html > > In v2 I’ve addressed the comments I’ve received from Kevin and Vladimir. > To this end, I’ve retained only the non-controversial part in

Re: [PATCH v4 03/25] assertions for block global state API

2021-11-11 Thread Hanna Reitz
On 25.10.21 12:17, Emanuele Giuseppe Esposito wrote: All the global state (GS) API functions will check that qemu_in_main_thread() returns true. If not, it means that the safety of BQL cannot be guaranteed, and they need to be moved to I/O. Signed-off-by: Emanuele Giuseppe Esposito

Re: [PATCH v2 0/3] virtio: increase VIRTQUEUE_MAX_SIZE to 32k

2021-11-11 Thread Stefan Hajnoczi
On Wed, Nov 10, 2021 at 04:53:33PM +0100, Christian Schoenebeck wrote: > On Mittwoch, 10. November 2021 16:14:19 CET Stefan Hajnoczi wrote: > > On Wed, Nov 10, 2021 at 02:14:43PM +0100, Christian Schoenebeck wrote: > > > On Mittwoch, 10. November 2021 11:05:50 CET Stefan Hajnoczi wrote: > > > As

[PATCH v2 1/1] Jobs based on custom runners: add CentOS Stream 8

2021-11-11 Thread Cleber Rosa
This introduces three different parts of a job designed to run on a custom runner managed by Red Hat. The goals include: a) propose a model for other organizations that want to onboard their own runners, with their specific platforms, build configuration and tests. b) bring

[PATCH v2 0/1] Jobs based on custom runners: add CentOS Stream 8

2021-11-11 Thread Cleber Rosa
This adds a new custom runner, showing an example of how other entities can add their own custom jobs to the GitLab CI pipeline. The runner (the machine and job) is to be managed by Red Hat, and adds, at the very least, bare metal x86_64 KVM testing capabilities to the QEMU pipeline. This brings

[PATCH-for-7.0 2/2] hw/nvme/ctrl: Pass buffers as 'void *' types

2021-11-11 Thread Philippe Mathieu-Daudé
These buffers can be anything, not an array of chars, so use the 'void *' type for them. Signed-off-by: Philippe Mathieu-Daudé --- hw/nvme/nvme.h | 4 ++-- hw/nvme/ctrl.c | 10 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h index

[PATCH-for-7.0 0/2] hw/nvme/ctrl: Buffer types cleanups

2021-11-11 Thread Philippe Mathieu-Daudé
Some trivial notes I took while reviewing CVE-2021-3947: https://lore.kernel.org/qemu-devel/2021153125.2258176-1-phi...@redhat.com/ Based-on: <2021153125.2258176-1-phi...@redhat.com> *** BLURB HERE *** Philippe Mathieu-Daudé (2): hw/nvme/ctrl: Have nvme_addr_write() take const buffer

[PATCH-for-7.0 1/2] hw/nvme/ctrl: Have nvme_addr_write() take const buffer

2021-11-11 Thread Philippe Mathieu-Daudé
The 'buf' argument is not modified, so better pass it as const type. Signed-off-by: Philippe Mathieu-Daudé --- hw/nvme/ctrl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 634b290e069..c7cce63372a 100644 --- a/hw/nvme/ctrl.c +++

[PATCH 09/10] vhost: stick to -errno error return convention

2021-11-11 Thread Roman Kagan
The generic vhost code expects that many of the VhostOps methods in the respective backends set errno on errors. However, none of the existing backends actually bothers to do so. In a number of those methods errno from the failed call is clobbered by successful later calls to some library

[PATCH 08/10] vhost-user: stick to -errno error return convention

2021-11-11 Thread Roman Kagan
VhostOps methods in user_ops are not very consistent in their error returns: some return negated errno while others just -1. Make sure all of them consistently return negated errno. This also helps error propagation from the functions being called inside. Besides, this synchronizes the error

[PATCH 07/10] vhost-vdpa: stick to -errno error return convention

2021-11-11 Thread Roman Kagan
Almost all VhostOps methods in vdpa_ops follow the convention of returning negated errno on error. Adjust the few that don't. To that end, rework vhost_vdpa_add_status to check if setting of the requested status bits has succeeded and return the respective error code it hasn't, and propagate the

[PATCH 04/10] chardev/char-fe: don't allow EAGAIN from blocking read

2021-11-11 Thread Roman Kagan
As its name suggests, ChardevClass.chr_sync_read is supposed to do a blocking read. The only implementation of it, tcp_chr_sync_read, does set the underlying io channel to the blocking mode indeed. Therefore a failure return with EAGAIN is not expected from this call. So do not retry it in

[PATCH 06/10] vhost-backend: stick to -errno error return convention

2021-11-11 Thread Roman Kagan
Almost all VhostOps methods in kernel_ops follow the convention of returning negated errno on error. Adjust the only one that doesn't. Signed-off-by: Roman Kagan --- hw/virtio/vhost-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/virtio/vhost-backend.c

[PATCH 10/10] vhost-user-blk: propagate error return from generic vhost

2021-11-11 Thread Roman Kagan
Fix the only callsite that doesn't propagate the error code from the generic vhost code. Signed-off-by: Roman Kagan --- hw/block/vhost-user-blk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index

[PATCH 00/10] vhost: stick to -errno error return convention

2021-11-11 Thread Roman Kagan
Error propagation between the generic vhost code and the specific backends is not quite consistent: some places follow "return -1 and set errno" convention, while others assume "return negated errno". Furthermore, not enough care is taken not to clobber errno. As a result, on certain code paths

[PATCH 02/10] chardev/char-socket: tcp_chr_recv: don't clobber errno

2021-11-11 Thread Roman Kagan
tcp_chr_recv communicates the specific error condition to the caller via errno. However, after setting it, it may call into some system calls or library functions which can clobber the errno. Avoid this by moving the errno assignment to the end of the function. Signed-off-by: Roman Kagan ---

[PATCH 03/10] chardev/char-socket: tcp_chr_sync_read: don't clobber errno

2021-11-11 Thread Roman Kagan
After the return from tcp_chr_recv, tcp_chr_sync_read calls into a function which eventually makes a system call and may clobber errno. Make a copy of errno right after tcp_chr_recv and restore the errno on return from tcp_chr_sync_read. Signed-off-by: Roman Kagan --- chardev/char-socket.c | 3

[PATCH 01/10] vhost-user-blk: reconnect on any error during realize

2021-11-11 Thread Roman Kagan
vhost-user-blk realize only attempts to reconnect if the previous connection attempt failed on "a problem with the connection and not an error related to the content (which would fail again the same way in the next attempt)". However this distinction is very subtle, and may be inadvertently

[PATCH 05/10] vhost-backend: avoid overflow on memslots_limit

2021-11-11 Thread Roman Kagan
Fix the (hypothetical) potential problem when the value parsed out of the vhost module parameter in sysfs overflows the return value from vhost_kernel_memslots_limit. Signed-off-by: Roman Kagan --- hw/virtio/vhost-backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git

[PATCH-for-6.2] hw/nvme/ctrl: Fix buffer overrun (CVE-2021-3947)

2021-11-11 Thread Philippe Mathieu-Daudé
Both 'buf_len' and 'off' arguments are under guest control. Since nvme_c2h() doesn't check out of boundary access, the caller must check for eventual buffer overrun on 'trans_len'. Cc: qemu-sta...@nongnu.org Reported-by: Qiuhao Li Fixes: f432fdfa121 ("support changed namespace asynchronous

Re: [PATCH v4 02/25] include/block/block: split header into I/O and global state API

2021-11-11 Thread Hanna Reitz
On 25.10.21 12:17, Emanuele Giuseppe Esposito wrote: block.h currently contains a mix of functions: some of them run under the BQL and modify the block layer graph, others are instead thread-safe and perform I/O in iothreads. It is not easy to understand which function is part of which group

[PATCH v2 10/10] iotests/030: Unthrottle parallel jobs in reverse

2021-11-11 Thread Hanna Reitz
See the comment for why this is necessary. Signed-off-by: Hanna Reitz --- tests/qemu-iotests/030 | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index 5fb65b4bef..567bf1da67 100755 --- a/tests/qemu-iotests/030 +++

[PATCH v2 09/10] block: Let replace_child_noperm free children

2021-11-11 Thread Hanna Reitz
In most of the block layer, especially when traversing down from other BlockDriverStates, we assume that BdrvChild.bs can never be NULL. When it becomes NULL, it is expected that the corresponding BdrvChild pointer also becomes NULL and the BdrvChild object is freed. Therefore, once

[PATCH v2 08/10] block: Let replace_child_tran keep indirect pointer

2021-11-11 Thread Hanna Reitz
As of a future commit, bdrv_replace_child_noperm() will clear the indirect BdrvChild pointer passed to it if the new child BDS is NULL. bdrv_replace_child_tran() will want to let it do that, but revert this change in its abort handler. For that, we need to have it receive a BdrvChild ** pointer,

[PATCH v2 04/10] block: Drop detached child from ignore list

2021-11-11 Thread Hanna Reitz
bdrv_attach_child_common_abort() restores the parent's AioContext. To do so, the child (which was supposed to be attached, but is now detached again by this abort handler) is added to the ignore list for the AioContext changing functions. However, since we modify a BDS's children list in the

[PATCH v2 02/10] block: Manipulate children list in .attach/.detach

2021-11-11 Thread Hanna Reitz
The children list is specific to BDS parents. We should not modify it in the general children modification code, but let BDS parents deal with it in their .attach() and .detach() methods. This also has the advantage that a BdrvChild is removed from the children list before its .bs pointer can

[PATCH v2 03/10] block: Unite remove_empty_child and child_free

2021-11-11 Thread Hanna Reitz
Now that bdrv_remove_empty_child() no longer removes the child from the parent's children list but only checks that it is not in such a list, it is only a wrapper around bdrv_child_free() that checks that the child is empty and unused. That should apply to all children that we free, so put those

[PATCH v2 01/10] stream: Traverse graph after modification

2021-11-11 Thread Hanna Reitz
bdrv_cor_filter_drop() modifies the block graph. That means that other parties can also modify the block graph before it returns. Therefore, we cannot assume that the result of a graph traversal we did before remains valid afterwards. We should thus fetch `base` and `unfiltered_base` afterwards

[PATCH v2 05/10] block: Pass BdrvChild ** to replace_child_noperm

2021-11-11 Thread Hanna Reitz
bdrv_replace_child_noperm() modifies BdrvChild.bs, and can potentially set it to NULL. That is dangerous, because BDS parents generally assume that their children's .bs pointer is never NULL. We therefore want to let bdrv_replace_child_noperm() set the corresponding BdrvChild pointer to NULL,

[PATCH v2 07/10] transactions: Invoke clean() after everything else

2021-11-11 Thread Hanna Reitz
Invoke the transaction drivers' .clean() methods only after all .commit() or .abort() handlers are done. This makes it easier to have nested transactions where the top-level transactions pass objects to lower transactions that the latter can still use throughout their commit/abort phases, while

[PATCH v2 06/10] block: Restructure remove_file_or_backing_child()

2021-11-11 Thread Hanna Reitz
As of a future patch, bdrv_replace_child_tran() will take a BdrvChild ** pointer. Prepare for that by getting such a pointer and using it where applicable, and (dereferenced) as a parameter for bdrv_replace_child_tran(). Signed-off-by: Hanna Reitz --- block.c | 21 - 1 file

[PATCH v2 00/10] block: Attempt on fixing 030-reported errors

2021-11-11 Thread Hanna Reitz
Hi, v1 cover letter: https://lists.nongnu.org/archive/html/qemu-devel/2021-11/msg01287.html In v2 I’ve addressed the comments I’ve received from Kevin and Vladimir. To this end, I’ve retained only the non-controversial part in patch 5, and split everything else (i.e. the stuff relating to

Re: [PATCH v9 8/8] hmp: add virtio commands

2021-11-11 Thread Jonah Palmer
On 11/10/21 08:30, Markus Armbruster wrote: Jonah Palmer writes: From: Laurent Vivier This patch implements the HMP versions of the virtio QMP commands. Signed-off-by: Jonah Palmer --- hmp-commands-info.hx | 218 ++ include/monitor/hmp.h | 5 +

Re: [PATCH v9 7/8] qmp: add QMP command x-query-virtio-queue-element

2021-11-11 Thread Jonah Palmer
On 11/10/21 08:52, Markus Armbruster wrote: Jonah Palmer writes: From: Laurent Vivier This new command shows the information of a VirtQueue element. Signed-off-by: Jonah Palmer [...] diff --git a/qapi/virtio.json b/qapi/virtio.json index 0f65044..c57fbc5 100644 --- a/qapi/virtio.json

Re: [PATCH v9 5/8] qmp: decode feature & status bits in virtio-status

2021-11-11 Thread Jonah Palmer
On 11/10/21 08:49, Markus Armbruster wrote: Jonah Palmer writes: From: Laurent Vivier Display feature names instead of bitmaps for host, guest, and backend for VirtIODevice. Display status names instead of bitmaps for VirtIODevice. Display feature names instead of bitmaps for backend,

Re: [PATCH v9 4/8] qmp: add QMP command x-query-virtio-status

2021-11-11 Thread Jonah Palmer
On 11/10/21 08:08, Markus Armbruster wrote: Jonah Palmer writes: From: Laurent Vivier This new command shows the status of a VirtIODevice, including its corresponding vhost device status (if active). Next patch will improve output by decoding feature bits, including vhost device's feature

Re: [PATCH v9 3/8] qmp: add QMP command x-query-virtio

2021-11-11 Thread Jonah Palmer
On 11/10/21 07:03, Markus Armbruster wrote: Jonah Palmer writes: From: Laurent Vivier This new command lists all the instances of VirtIODevice with their QOM paths and virtio type/name. Signed-off-by: Jonah Palmer [...] diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json index