Because the CMB BAR has a min_access_size of 2, if you read the last
byte it will try to memcpy *2* bytes from n->cmbuf, causing an off-by-one
error. This is CVE-2018-16847.
Another way to fix this might be to register the CMB as a RAM memory
region, which would also be more efficient. However,
On 16 November 2018 at 03:28, John Snow wrote:
> I looked again. I think Vladimir's patch will shut up Coverity for sure,
> feel free to apply it if you want this out of your hair.
>
> Stefan suggests the following, however;
>
>
> diff --git a/migration/block-dirty-bitmap.c
On 14.11.18 14:54, Alberto Garcia wrote:
> On Thu 11 Oct 2018 09:21:34 AM CEST, Fam Zheng wrote:
>> The lock_fd field is not strictly necessary because transferring locked
>> bytes from old fd to the new one shouldn't fail anyway. This spares the
>> user one fd per image.
>>
>> Signed-off-by: Fam
Paolo Bonzini 于2018年11月16日周五 下午5:31写道：
> Because the CMB BAR has a min_access_size of 2, if you read the last
> byte it will try to memcpy *2* bytes from n->cmbuf, causing an off-by-one
> error. This is CVE-2018-16847.
>
> Another way to fix this might be to register the CMB as a RAM memory
>
On 15.11.18 15:28, Alberto Garcia wrote:
> On Wed 14 Nov 2018 06:24:10 PM CET, Max Reitz wrote:
Permission system
=
GRAPH_MOD
-
We need some way for the commit job to prevent graph changes on its
chain while it is running. Our
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20181116093152.27227-1-pbonz...@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH for-3.1] nvme: fix out-of-bounds access to the CMB
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
On Fri 16 Nov 2018 02:34:25 PM CET, Max Reitz wrote:
> To me that looks like a problem in the general reopen code.
> raw_reopen_prepare() is called and succeeds. Then
> bdrv_reopen_prepare() notices the option wasn't handled and therefore
> fails. bdrv_reopen_multiple() thus doesn't set
On 16.11.18 14:58, Alberto Garcia wrote:
> On Fri 16 Nov 2018 02:34:25 PM CET, Max Reitz wrote:
>> To me that looks like a problem in the general reopen code.
>> raw_reopen_prepare() is called and succeeds. Then
>> bdrv_reopen_prepare() notices the option wasn't handled and therefore
>> fails.
On 16.11.18 15:02, Max Reitz wrote:
> On 16.11.18 14:58, Alberto Garcia wrote:
>> On Fri 16 Nov 2018 02:34:25 PM CET, Max Reitz wrote:
>>> To me that looks like a problem in the general reopen code.
>>> raw_reopen_prepare() is called and succeeds. Then
>>> bdrv_reopen_prepare() notices the option
On Tue, Oct 16, 2018 at 04:20:18PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Theoretically possible that we finish the skipping loop with bs = NULL
> and the following code will crash trying to dereference it. Fix that.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
>
Hi again,
I hit another occurrence of this issue and collected the qemy dump as well. So
at the moment I have two qemu dumps for this issue happening on two different
guests.
I wish I know how to analyze them myself but is not the case so if anyone in
the list is willing to take a look I'm
On Fri 16 Nov 2018 04:03:12 PM CET, Alberto Garcia wrote:
>> top (T) -> intermediate (I) -> base (B)
>>
>> Now you commit from top to base. Clearly you don't want the backing
>> chain between top and base to change. So say you unshare the GRAPH_MOD
>> permission (implying that whenever a parent
On 16.11.18 16:18, Kevin Wolf wrote:
> Am 16.11.2018 um 16:03 hat Alberto Garcia geschrieben:
>>> I don't think anything needs a way to generally block graph changes
>>> around some node. We only need to prevent changes to very specific
>>> sets of edges. This is something that the permission
This does two minor fixes to the NBD code and adds significant coverage
of the NBD TLS support to detect future problems.
The first two patches should be for 3.1.
The tests can wait till 4.0 if desired.
Daniel P. Berrangé (6):
nbd: fix whitespace in server error message
nbd: stop waiting
bdrv_reopen_multiple() does not invoke bdrv_reopen_abort() for the
element of the reopen queue for which bdrv_reopen_prepare() failed,
because it assumes that the prepare function will have rolled back all
changes already.
However, bdrv_reopen_prepare() does not do this in every case: It may
(I just wanted to reply quickly to this point, I'll read the rest of the
e-mail later)
On Fri 16 Nov 2018 05:34:08 PM CET, Max Reitz wrote:
> > GRAPH_MOD with the meaning that Berto suggested isn't weird or
> > complicated to understand. It's only the wrong tool because it
> > blocks more than
I feel bad for trimming your mail so much, but that's just because I've
read it and I agree. So, it's trimming of the good kind.
(Then again, when is trimming of a long mail ever bad?)
On 16.11.18 18:13, Kevin Wolf wrote:
[...]
> What we noted down about counters was more meaningful progress
These are fixes for issues I found when looking after something Berto
has reported. The second patch fixes that issue Berto found, the first
one is only kind of related.
For the first patch: bdrv_reopen_abort() or bdrv_reopen_commit() are
called only if the respective bdrv_reopen_prepare() has
Am 15.11.2018 um 19:34 hat Eric Blake geschrieben:
> Although off_t permits up to 63 bits (8EB) of file offsets, in
> practice, we're going to hit other limits first. Document some
> of those limits in the qcow2 spec (some are inherent, others are
> implementation choices of qemu), and how choice
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote:
The helpers for starting/stopping qemu-nbd in 058 will be useful in
other test cases, so move them into a common.nbd file.
In fact, I already have a proposed patch 228 that I'm trying to clean up
for potential inclusion in 3.1 that could use
On 16.11.18 17:02, Alberto Garcia wrote:
> On Fri 16 Nov 2018 04:53:00 PM CET, Max Reitz wrote:
>> bdrv_reopen_multiple() does not invoke bdrv_reopen_abort() for the
>> element of the reopen queue for which bdrv_reopen_prepare() failed,
>> because it assumes that the prepare function will have
Signed-off-by: Max Reitz
---
tests/qemu-iotests/182 | 71 ++
tests/qemu-iotests/182.out | 9 +
2 files changed, 80 insertions(+)
diff --git a/tests/qemu-iotests/182 b/tests/qemu-iotests/182
index 4b31592fb8..3b7689c1d5 100755
---
The helpers for starting/stopping qemu-nbd in 058 will be useful in
other test cases, so move them into a common.nbd file.
Signed-off-by: Daniel P. Berrangé
---
tests/qemu-iotests/058| 47 +
tests/qemu-iotests/common.nbd | 56
On 16 November 2018 at 16:16, John Snow wrote:
> On 11/16/18 5:04 AM, Peter Maydell wrote:
>> Personally I think the main benefit of this sort of Coverity
>> warning is that it nudges you to work through and figure out
>> whether something really can be NULL or not. Stefan's fix
>> will satisfy
Signed-off-by: Max Reitz
---
tests/qemu-iotests/182 | 71 ++
tests/qemu-iotests/182.out | 9 +
2 files changed, 80 insertions(+)
diff --git a/tests/qemu-iotests/182 b/tests/qemu-iotests/182
index 4b31592fb8..3b7689c1d5 100755
---
s->locked_shared_perm is the set of bits locked in the file, which is
the inverse of the permissions actually shared. So we need to pass them
as they are to raw_apply_lock_bytes() instead of inverting them again.
Reported-by: Alberto Garcia
Signed-off-by: Max Reitz
---
block/file-posix.c | 2
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote:
A space was missing after the option number was printed:
Option 0x8not permitted before TLS
becomes
Option 0x8 not permitted before TLS
This fixes
commit 3668328303429f3bc93ab3365c66331600b06a2d
Author: Eric Blake
Date: Fri
bdrv_reopen_multiple() does not invoke bdrv_reopen_abort() for the
element of the reopen queue for which bdrv_reopen_prepare() failed,
because it assumes that the prepare function will have rolled back all
changes already.
However, bdrv_reopen_prepare() does not do this in every case: It may
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote:
When sending a NBD_CMD_DISC message there is no reply expected,
however, the nbd_read_eof() coroutine is still waiting for a reply.
In a plain NBD connection this doesn't matter as it will just get an
EOF, however, on a TLS connection it will get an
Am 15.11.2018 um 17:28 hat Eric Blake geschrieben:
> On 11/15/18 9:45 AM, Kevin Wolf wrote:
> > Am 15.11.2018 um 03:03 hat Eric Blake geschrieben:
> > > This change has no semantic impact: all drivers either leave the
> > > value at 0 (no inherent 32-bit limit is still translated into
> > >
Am 15.11.2018 um 23:27 hat Nir Soffer geschrieben:
> On Sun, Nov 11, 2018 at 6:11 PM Nir Soffer wrote:
>
> > On Wed, Nov 7, 2018 at 7:55 PM Nir Soffer wrote:
> >
> >> On Wed, Nov 7, 2018 at 7:27 PM Kevin Wolf wrote:
> >>
> >>> Am 07.11.2018 um 15:56 hat Nir Soffer geschrieben:
> >>> > Wed, Nov
Am 16.11.2018 um 16:27 hat Max Reitz geschrieben:
> On 16.11.18 16:18, Kevin Wolf wrote:
> > Am 16.11.2018 um 16:03 hat Alberto Garcia geschrieben:
> >>> I don't think anything needs a way to generally block graph changes
> >>> around some node. We only need to prevent changes to very specific
>
On 11/16/18 9:32 AM, Kevin Wolf wrote:
Am 15.11.2018 um 17:28 hat Eric Blake geschrieben:
On 11/15/18 9:45 AM, Kevin Wolf wrote:
Am 15.11.2018 um 03:03 hat Eric Blake geschrieben:
This change has no semantic impact: all drivers either leave the
value at 0 (no inherent 32-bit limit is still
Add helpers to common.tls for creating TLS certificates for a CA,
server and client.
Signed-off-by: Daniel P. Berrangé
---
tests/qemu-iotests/common.tls | 139 ++
1 file changed, 139 insertions(+)
create mode 100644 tests/qemu-iotests/common.tls
diff --git
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote:
If the qemu-nbd UNIX socket has not shown up, the tests will sleep a bit
and then check again repeatedly for upto 30 seconds. This is pointless
s/upto/up to/
if the qemu-nbd process has quit due to an error, so check whether the
pid is still
When sending a NBD_CMD_DISC message there is no reply expected,
however, the nbd_read_eof() coroutine is still waiting for a reply.
In a plain NBD connection this doesn't matter as it will just get an
EOF, however, on a TLS connection it will get an interrupted TLS data
packet. The nbd_read_eof()
On 11/16/18 5:04 AM, Peter Maydell wrote:
> On 16 November 2018 at 03:28, John Snow wrote:
>> I looked again. I think Vladimir's patch will shut up Coverity for sure,
>> feel free to apply it if you want this out of your hair.
>>
>> Stefan suggests the following, however;
>>
>>
>> diff --git
Am 16.11.2018 um 16:54 hat Eric Blake geschrieben:
> On 11/16/18 9:32 AM, Kevin Wolf wrote:
> > Am 15.11.2018 um 17:28 hat Eric Blake geschrieben:
> > > On 11/15/18 9:45 AM, Kevin Wolf wrote:
> > > > Am 15.11.2018 um 03:03 hat Eric Blake geschrieben:
> > > > > This change has no semantic impact:
On 11/16/18 3:41 PM, Eric Blake wrote:
+#!/bin/bash
I know we're using bash,
+
+function nbd_server_stop()
+{
+function nbd_server_wait_for_unix_socket()
and bash supports 'function', but it is an obsolete syntactic sugar
thing that I don't recommend using. (In ksh, it actually makes
From: Remy Noel
We no longer modify existing handlers entries and instead, always insert
those after having properly initialized those.
Also, we do not call aio_epoll_update for deleted handlers as this has
no impact whastoever.
Signed-off-by: Remy Noel
---
util/aio-posix.c | 85
From: Remy Noel
It is still used for bottom halves though and to avoid concurent
set_fd_handlers (We could probably decorrelate the two, but
set_fd_handlers are quite rare so it probably isn't worth it).
Signed-off-by: Remy Noel
---
include/block/aio.h | 4 +++-
util/aio-posix.c| 20
From: Remy Noel
get rid of the delete attribute.
We still need to get rid of the context list lock.
Signed-off-by: Remy Noel
---
util/aio-posix.c | 75 ++--
util/aio-win32.c | 43 ++-
2 files changed, 49 insertions(+), 69
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote:
The helpers for starting/stopping qemu-nbd in 058 will be useful in
other test cases, so move them into a common.nbd file.
Signed-off-by: Daniel P. Berrangé
---
tests/qemu-iotests/058| 47 +
Bash allows functions to be declared with or without the leading
keyword 'function'; but including the keyword does not comply with
POSIX syntax, and is confusing to ksh users where the use of the
keyword changes the scoping rules for functions. Stick to the
POSIX form through iotests.
Done
On Fri 16 Nov 2018 01:14:37 PM CET, Max Reitz wrote:
> Permission system
> =
>
> GRAPH_MOD
> -
>
> We need some way for the commit job to prevent graph changes on its
> chain while it is running. Our current blocker doesn’t do the job,
>
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote:
Add helpers to common.tls for creating TLS certificates for a CA,
server and client.
MUCH appreciated! We NEED this coverage, easily automated.
Signed-off-by: Daniel P. Berrangé
---
tests/qemu-iotests/common.tls | 139
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote:
Add tests that validate it is possible to connect to an NBD server
running TLS mode. Also test mis-matched TLS vs non-TLS connections
correctly fail.
---
tests/qemu-iotests/233| 99 +++
Coverity warns that backing_bs() could give us a NULL pointer, which
we then use without checking that it isn't.
In our loop condition, we check bs && bs->drv as a point of habit, but
by nature of the block graph, we cannot have null bs pointers here.
This loop skips only implicit nodes, which
On 16.11.18 16:03, Alberto Garcia wrote:
> On Fri 16 Nov 2018 01:14:37 PM CET, Max Reitz wrote:
>> Permission system
>> =
>>
>> GRAPH_MOD
>> -
>>
>> We need some way for the commit job to prevent graph changes on its
>> chain while it is
Am 16.11.2018 um 16:03 hat Alberto Garcia geschrieben:
> > I don't think anything needs a way to generally block graph changes
> > around some node. We only need to prevent changes to very specific
> > sets of edges. This is something that the permission system just
> > cannot do.
>
> But what
s->locked_shared_perm is the set of bits locked in the file, which is
the inverse of the permissions actually shared. So we need to pass them
as they are to raw_apply_lock_bytes() instead of inverting them again.
Reported-by: Alberto Garcia
Signed-off-by: Max Reitz
---
block/file-posix.c | 2
A space was missing after the option number was printed:
Option 0x8not permitted before TLS
becomes
Option 0x8 not permitted before TLS
This fixes
commit 3668328303429f3bc93ab3365c66331600b06a2d
Author: Eric Blake
Date: Fri Oct 14 13:33:09 2016 -0500
nbd: Send message along
If the qemu-nbd UNIX socket has not shown up, the tests will sleep a bit
and then check again repeatedly for upto 30 seconds. This is pointless
if the qemu-nbd process has quit due to an error, so check whether the
pid is still alive before waiting and retrying.
Signed-off-by: Daniel P. Berrangé
Add tests that validate it is possible to connect to an NBD server
running TLS mode. Also test mis-matched TLS vs non-TLS connections
correctly fail.
---
tests/qemu-iotests/233| 99 +++
tests/qemu-iotests/233.out| 33
On Fri 16 Nov 2018 04:53:00 PM CET, Max Reitz wrote:
> bdrv_reopen_multiple() does not invoke bdrv_reopen_abort() for the
> element of the reopen queue for which bdrv_reopen_prepare() failed,
> because it assumes that the prepare function will have rolled back all
> changes already.
>
> However,
On 16.11.18 16:51, Kevin Wolf wrote:
> Am 16.11.2018 um 16:27 hat Max Reitz geschrieben:
>> On 16.11.18 16:18, Kevin Wolf wrote:
>>> Am 16.11.2018 um 16:03 hat Alberto Garcia geschrieben:
> I don't think anything needs a way to generally block graph changes
> around some node. We only
These are fixes for issues I found when looking after something Berto
has reported. The second patch fixes that issue Berto found, the first
one is only kind of related.
For the first patch: bdrv_reopen_abort() or bdrv_reopen_commit() are
called only if the respective bdrv_reopen_prepare() has
Am 16.11.2018 um 17:34 hat Max Reitz geschrieben:
> On 16.11.18 16:51, Kevin Wolf wrote:
> > Am 16.11.2018 um 16:27 hat Max Reitz geschrieben:
> >> On 16.11.18 16:18, Kevin Wolf wrote:
> >>> Am 16.11.2018 um 16:03 hat Alberto Garcia geschrieben:
> > I don't think anything needs a way to
58 matches
Mail list logo
