Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-01 Thread Markus Armbruster
Paolo Bonzini writes: > On 30/11/20 16:30, Daniel P. Berrangé wrote: >> { 'struct': 'QCryptoSecretCommon', >>'base': 'Object', >>'state': { 'rawdata': '*uint8_t', >> 'rawlen': 'size_t' }, >>'data': { '*format': 'QCryptoSecretFormat', >> '*keyid': 'str', >>

Re: [PATCH v2] hw/nvme: Move NVMe emulation out of hw/block/ directory

2020-12-01 Thread Philippe Mathieu-Daudé
On 11/30/20 9:52 PM, Klaus Jensen wrote: > On Nov 30 15:52, Philippe Mathieu-Daudé wrote: >> As IDE used to be, NVMe emulation is becoming an active >> subsystem. Move it into its own namespace. >> >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> v2: Rebased after nvme-ns got merged in commit 86

qemu 6.0 rbd driver rewrite

2020-12-01 Thread Peter Lieven
Hi, i would like to submit a series for 6.0 which will convert the aio hooks to native coroutine hooks and add write zeroes support. The aio routines are nowadays just an emulation on top of coroutines which add additional overhead. For this I would like to lift the minimum librbd requirement

[PATCH 0/3] hw/scsi/megasas: Avoid buffer overrun in megasas_handle_scsi()

2020-12-01 Thread Philippe Mathieu-Daudé
FWIW megasas is not use by KVM. Not sure what is the proper fix, but at least we have a reproducer. We might improve "scsi/utils" by adding length argument to scsi_cdb_length() and check valid there, but this will take time (large refactor). Add assertions there is too violent. Philippe Mathieu-

[PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-01 Thread Philippe Mathieu-Daudé
cdb_len can not be zero... (or less than 6) here, else we have a out-of-bound read first in scsi_cdb_length(): 71 int scsi_cdb_length(uint8_t *buf) 72 { 73 int cdb_len; 74 75 switch (buf[0] >> 5) { 76 case 0: 77 cdb_len = 6; 78 break; Then another out-of-bound

[PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done

2020-12-01 Thread Philippe Mathieu-Daudé
Missed in fd250172842 ("qtest: add a reproducer for LP#1878642"). Signed-off-by: Philippe Mathieu-Daudé --- tests/qtest/fuzz-test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c index 9cb4c42bdea..87b72307a5b 100644 --- a/tests/qtest/fuzz-t

[RFC PATCH 3/3] hw/scsi/megasas: Have incorrect cdb return MFI_STAT_ABORT_NOT_POSSIBLE

2020-12-01 Thread Philippe Mathieu-Daudé
Avoid out-of-bound array access with invalid CDB is provided. Signed-off-by: Philippe Mathieu-Daudé --- RFC because I have no clue how hardware works. Maybe returning MFI_STAT_ARRAY_INDEX_INVALID is better? Do we need to call megasas_write_sense()? hw/scsi/megasas.c | 7 ++- 1 file changed,

Re: [PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done

2020-12-01 Thread Thomas Huth
On 01/12/2020 16.13, Philippe Mathieu-Daudé wrote: > Missed in fd250172842 ("qtest: add a reproducer for LP#1878642"). > > Signed-off-by: Philippe Mathieu-Daudé > --- > tests/qtest/fuzz-test.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-te

Re: [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-01 Thread Thomas Huth
On 01/12/2020 16.13, Philippe Mathieu-Daudé wrote: > cdb_len can not be zero... (or less than 6) here, else we have a > out-of-bound read first in scsi_cdb_length(): > > 71 int scsi_cdb_length(uint8_t *buf) > 72 { > 73 int cdb_len; > 74 > 75 switch (buf[0] >> 5) { > 76 case 0: >

Re: [PATCH v3 00/17] 64bit block-layer

2020-12-01 Thread Vladimir Sementsov-Ogievskiy
Hi! I'm sorry, I should have pinged it, or resend, or suggest to pull at least a half long ago :( I've rebased it on master and make some fixes. What to do next? I can just resend. But I'm afraid that Eric's careful audits may be out of date: time passed, there is no guarantee that callers ar

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-01 Thread Kevin Wolf
Am 30.11.2020 um 20:35 hat Paolo Bonzini geschrieben: > On 30/11/20 19:10, Kevin Wolf wrote: > > Am 30.11.2020 um 17:57 hat Paolo Bonzini geschrieben: > > > The main problem is that it wouldn't extend well, if at all, to > > > machines and devices. So those would still not be integrated into the >

Re: [PATCH v11 0/7] Introduce 'yank' oob qmp command to recover from hanging qemu

2020-12-01 Thread Markus Armbruster
Lukas Straub writes: > Hello Everyone, > So here is v11. > @Eric Blake and @Marc-André Lureau: We still need ACKs for NBD and chardev. Once we have them, I can take the series through my tree.

Re: [PATCH v3 00/17] 64bit block-layer

2020-12-01 Thread Vladimir Sementsov-Ogievskiy
01.12.2020 19:07, Vladimir Sementsov-Ogievskiy wrote: I have an idea: instead of auditing each function callers, can we just make some good assumptions (like that the whole offset/bytes request being aligned to bs->request_alignement doesn't lay inside [0..INT64_MAX] region), check it once in

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-01 Thread Paolo Bonzini
On 01/12/20 17:20, Kevin Wolf wrote: Am 30.11.2020 um 20:35 hat Paolo Bonzini geschrieben: For devices it's just the practical issue that there are too many to have something like this series. For machine types the main issue is that the version-specific machine types would have to be defined i

[PATCH] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-01 Thread Alexander Bulekov
From: Philippe Mathieu-Daudé cdb_len can not be zero... (or less than 6) here, else we have a out-of-bound read first in scsi_cdb_length(): 71 int scsi_cdb_length(uint8_t *buf) 72 { 73 int cdb_len; 74 75 switch (buf[0] >> 5) { 76 case 0: 77 cdb_len = 6; 78 bre

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-01 Thread Eduardo Habkost
On Tue, Dec 01, 2020 at 06:16:14PM +0100, Paolo Bonzini wrote: > On 01/12/20 17:20, Kevin Wolf wrote: [...] > > BlockdevOptions is about external interfaces, not about > > implementation details. Same thing as QOM properties are external > > interfaces, not implementation details. There may be fiel

[PATCH v2 0/4] hw/scsi/megasas: Avoid buffer overrun in megasas_handle_scsi()

2020-12-01 Thread Philippe Mathieu-Daudé
FWIW megasas is not use by KVM. Not sure what is the proper fix, but at least we have a reproducer. Since v1: - Fix assert() condition - Extract reproducer in different patch for git-bisect (thuth) - Add simpler reproducer from Alex - Try better scsi error Philippe Mathieu-Daudé (4): tests/qte

[PATCH v2 3/4] tests/qtest/fuzz-test: Add test_megasas_cdb_len_zero() reproducer

2020-12-01 Thread Philippe Mathieu-Daudé
Add a reproducer which triggers (without the previous patch): $ make check-qtest-x86_64 Running test qtest-x86_64/fuzz-test qemu-system-x86_64: hw/scsi/megasas.c:1679: megasas_handle_scsi: Assertion `cdb_len > 0 && scsi_cdb_length(cdb) <= cdb_len' failed. tests/qtest/libqtest.c:181: kill_

[PATCH v2 1/4] tests/qtest/fuzz-test: Quit test_lp1878642 once done

2020-12-01 Thread Philippe Mathieu-Daudé
Missed in fd250172842 ("qtest: add a reproducer for LP#1878642"). Reviewed-by: Thomas Huth Signed-off-by: Philippe Mathieu-Daudé --- tests/qtest/fuzz-test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c index 9cb4c42bdea..87b72307a5b 10064

[PATCH v2 2/4] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

2020-12-01 Thread Philippe Mathieu-Daudé
cdb_len can not be zero... (or less than 6) here, else we have a out-of-bound read first in scsi_cdb_length(): 71 int scsi_cdb_length(uint8_t *buf) 72 { 73 int cdb_len; 74 75 switch (buf[0] >> 5) { 76 case 0: 77 cdb_len = 6; 78 break; Then another out-of-bound

Re: [PATCH v2 3/4] tests/qtest/fuzz-test: Add test_megasas_cdb_len_zero() reproducer

2020-12-01 Thread Philippe Mathieu-Daudé
On 12/1/20 8:10 PM, Philippe Mathieu-Daudé wrote: > Add a reproducer which triggers (without the previous patch): > > $ make check-qtest-x86_64 > Running test qtest-x86_64/fuzz-test > qemu-system-x86_64: hw/scsi/megasas.c:1679: megasas_handle_scsi: Assertion > `cdb_len > 0 && scsi_cdb_lengt

[RFC PATCH v2 4/4] hw/scsi/megasas: Have incorrect cdb return MFI_STAT_ABORT_NOT_POSSIBLE

2020-12-01 Thread Philippe Mathieu-Daudé
Avoid out-of-bound array access with invalid CDB is provided. Signed-off-by: Philippe Mathieu-Daudé --- RFC because no clue how hardware works --- hw/scsi/megasas.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index f5ad4425b

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-01 Thread Kevin Wolf
Am 01.12.2020 um 18:16 hat Paolo Bonzini geschrieben: > On 01/12/20 17:20, Kevin Wolf wrote: > > Am 30.11.2020 um 20:35 hat Paolo Bonzini geschrieben: > > > For devices it's just the practical issue that there are too many to have > > > something like this series. For machine types the main issue

Re: [PATCH] qemu-nbd: Fix a memleak in nbd_client_thread()

2020-12-01 Thread Eric Blake
On 12/1/20 12:13 AM, Alex Chen wrote: > When the qio_channel_socket_connect_sync() fails > we should goto 'out_socket' label to free the 'sioc' instead of > goto 'out' label. > In addition, now the 'out' label is useless, delete it. > > Reported-by: Euler Robot > Signed-off-by: Alex Chen > --- >

Re: [PATCH v11 1/7] Introduce yank feature

2020-12-01 Thread Eric Blake
On 11/15/20 5:36 AM, Lukas Straub wrote: > The yank feature allows to recover from hanging qemu by "yanking" "allows to $verb" is not idiomatic English, better is "allows $subject to verb" or "allows ${verb}ing". In this case, I suggest "The yank feature allows the recovery of a hung qemu by "yan

Re: [PATCH v11 2/7] block/nbd.c: Add yank feature

2020-12-01 Thread Eric Blake
On 11/15/20 5:36 AM, Lukas Straub wrote: > Register a yank function which shuts down the socket and sets > s->state = NBD_CLIENT_QUIT. This is the same behaviour as if an > error occured. occurred > > Signed-off-by: Lukas Straub > Acked-by: Stefan Hajnoczi > --- > block/nbd.c | 154 ++

Re: [PATCH v11 1/7] Introduce yank feature

2020-12-01 Thread Eric Blake
On 12/1/20 2:43 PM, Eric Blake wrote: > On 11/15/20 5:36 AM, Lukas Straub wrote: >> The yank feature allows to recover from hanging qemu by "yanking" > > "allows to $verb" is not idiomatic English, better is "allows $subject > to verb" or "allows ${verb}ing". In this case, I suggest "The yank > f

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-01 Thread Paolo Bonzini
On 01/12/20 20:35, Kevin Wolf wrote: Am 01.12.2020 um 18:16 hat Paolo Bonzini geschrieben: I don't think this is actually a new things. We already have types and commands declared with things like 'if': 'defined(TARGET_S390X)'. As far as I understand, QAPI generated files are already built per ta

Re: [PATCH v3 00/17] 64bit block-layer

2020-12-01 Thread Eric Blake
On 12/1/20 10:07 AM, Vladimir Sementsov-Ogievskiy wrote: > Hi! > > I'm sorry, I should have pinged it, or resend, or suggest to pull at > least a half long ago :( > > I've rebased it on master and make some fixes. > > What to do next? I can just resend. But I'm afraid that Eric's careful > audit

Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

2020-12-01 Thread Eduardo Habkost
On Tue, Dec 01, 2020 at 10:23:57PM +0100, Paolo Bonzini wrote: > On 01/12/20 20:35, Kevin Wolf wrote: > > Am 01.12.2020 um 18:16 hat Paolo Bonzini geschrieben: > > I don't think this is actually a new things. We already have types and > > commands declared with things like 'if': 'defined(TARGET_S39

Re: [PATCH] qemu-nbd: Fix a memleak in nbd_client_thread()

2020-12-01 Thread Alex Chen
On 2020/12/2 4:15, Eric Blake wrote: > On 12/1/20 12:13 AM, Alex Chen wrote: >> When the qio_channel_socket_connect_sync() fails >> we should goto 'out_socket' label to free the 'sioc' instead of >> goto 'out' label. >> In addition, now the 'out' label is useless, delete it. >> >> Reported-by: Eule

Re: [PATCH v2 3/4] tests/qtest/fuzz-test: Add test_megasas_cdb_len_zero() reproducer

2020-12-01 Thread Thomas Huth
On 01/12/2020 20.10, Philippe Mathieu-Daudé wrote: > Add a reproducer which triggers (without the previous patch): > > $ make check-qtest-x86_64 > Running test qtest-x86_64/fuzz-test > qemu-system-x86_64: hw/scsi/megasas.c:1679: megasas_handle_scsi: Assertion > `cdb_len > 0 && scsi_cdb_leng