* [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device
@ 2021-06-21 9:32 Philippe Mathieu-Daudé
2021-06-21 13:18 ` Fam Zheng
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-21 9:32 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Kevin Wolf, qemu-block, Michal Prívozník,
qemu-stable, Maxim Levitsky, Eric Auger, Alex Williamson,
Stefan Hajnoczi, Max Reitz, Philippe Mathieu-Daudé
When the NVMe block driver was introduced (see commit bdd6a90a9e5,
January 2018), Linux VFIO_IOMMU_MAP_DMA ioctl was only returning
-ENOMEM in case of error. The driver was correctly handling the
error path to recycle its volatile IOVA mappings.
To fix CVE-2019-3882, Linux commit 492855939bdb ("vfio/type1: Limit
DMA mappings per container", April 2019) added the -ENOSPC error to
signal the user exhausted the DMA mappings available for a container.
The block driver started to mis-behave:
qemu-system-x86_64: VFIO_MAP_DMA failed: No space left on device
(qemu)
(qemu) info status
VM status: paused (io-error)
(qemu) c
VFIO_MAP_DMA failed: No space left on device
qemu-system-x86_64: block/block-backend.c:1968: blk_get_aio_context: Assertion `ctx == blk->ctx' failed.
Fix by handling the new -ENOSPC error (when DMA mappings are
exhausted) without any distinction to the current -ENOMEM error,
so we don't change the behavior on old kernels where the CVE-2019-3882
fix is not present.
An easy way to reproduce this bug is to restrict the DMA mapping
limit (65535 by default) when loading the VFIO IOMMU module:
# modprobe vfio_iommu_type1 dma_entry_limit=666
Cc: qemu-stable@nongnu.org
Reported-by: Michal Prívozník <mprivozn@redhat.com>
Fixes: bdd6a90a9e5 ("block: Add VFIO based NVMe driver")
Buglink: https://bugs.launchpad.net/qemu/+bug/1863333
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/65
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: KISS checking both errors undistinguishedly (Maxim)
---
block/nvme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/nvme.c b/block/nvme.c
index 2b5421e7aa6..c3d2a49866c 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1030,7 +1030,7 @@ try_map:
r = qemu_vfio_dma_map(s->vfio,
qiov->iov[i].iov_base,
len, true, &iova);
- if (r == -ENOMEM && retry) {
+ if ((r == -ENOMEM || r == -ENOSPC) && retry) {
retry = false;
trace_nvme_dma_flush_queue_wait(s);
if (s->dma_map_count) {
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device
2021-06-21 9:32 [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device Philippe Mathieu-Daudé
@ 2021-06-21 13:18 ` Fam Zheng
2021-06-21 15:13 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2021-06-21 13:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, qemu-block, Michal Prívozník, qemu-stable,
qemu-devel, Maxim Levitsky, Eric Auger, Alex Williamson,
Stefan Hajnoczi, Max Reitz
> On 21 Jun 2021, at 10:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> When the NVMe block driver was introduced (see commit bdd6a90a9e5,
> January 2018), Linux VFIO_IOMMU_MAP_DMA ioctl was only returning
> -ENOMEM in case of error. The driver was correctly handling the
> error path to recycle its volatile IOVA mappings.
>
> To fix CVE-2019-3882, Linux commit 492855939bdb ("vfio/type1: Limit
> DMA mappings per container", April 2019) added the -ENOSPC error to
> signal the user exhausted the DMA mappings available for a container.
>
> The block driver started to mis-behave:
>
> qemu-system-x86_64: VFIO_MAP_DMA failed: No space left on device
> (qemu)
> (qemu) info status
> VM status: paused (io-error)
> (qemu) c
> VFIO_MAP_DMA failed: No space left on device
> qemu-system-x86_64: block/block-backend.c:1968: blk_get_aio_context: Assertion `ctx == blk->ctx' failed.
Hi Phil,
The diff looks good to me, but I’m not sure what exactly caused the assertion failure. There is `if (r) { goto fail; }` that handles -ENOSPC before, so it should be treated as a general case. What am I missing?
Reviewed-by: Fam Zheng <fam@euphon.net>
>
> Fix by handling the new -ENOSPC error (when DMA mappings are
> exhausted) without any distinction to the current -ENOMEM error,
> so we don't change the behavior on old kernels where the CVE-2019-3882
> fix is not present.
>
> An easy way to reproduce this bug is to restrict the DMA mapping
> limit (65535 by default) when loading the VFIO IOMMU module:
>
> # modprobe vfio_iommu_type1 dma_entry_limit=666
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Michal Prívozník <mprivozn@redhat.com>
> Fixes: bdd6a90a9e5 ("block: Add VFIO based NVMe driver")
> Buglink: https://bugs.launchpad.net/qemu/+bug/1863333
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/65
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> v2: KISS checking both errors undistinguishedly (Maxim)
> ---
> block/nvme.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/nvme.c b/block/nvme.c
> index 2b5421e7aa6..c3d2a49866c 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -1030,7 +1030,7 @@ try_map:
> r = qemu_vfio_dma_map(s->vfio,
> qiov->iov[i].iov_base,
> len, true, &iova);
> - if (r == -ENOMEM && retry) {
> + if ((r == -ENOMEM || r == -ENOSPC) && retry) {
> retry = false;
> trace_nvme_dma_flush_queue_wait(s);
> if (s->dma_map_count) {
> --
> 2.31.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device
2021-06-21 13:18 ` Fam Zheng
@ 2021-06-21 15:13 ` Philippe Mathieu-Daudé
2021-06-21 15:36 ` Fam Zheng
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-21 15:13 UTC (permalink / raw)
To: Fam Zheng
Cc: Kevin Wolf, qemu-block, Michal Prívozník, qemu-stable,
qemu-devel, Maxim Levitsky, Eric Auger, Alex Williamson,
Stefan Hajnoczi, Max Reitz
On 6/21/21 3:18 PM, Fam Zheng wrote:
>
>
>> On 21 Jun 2021, at 10:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> When the NVMe block driver was introduced (see commit bdd6a90a9e5,
>> January 2018), Linux VFIO_IOMMU_MAP_DMA ioctl was only returning
>> -ENOMEM in case of error. The driver was correctly handling the
>> error path to recycle its volatile IOVA mappings.
>>
>> To fix CVE-2019-3882, Linux commit 492855939bdb ("vfio/type1: Limit
>> DMA mappings per container", April 2019) added the -ENOSPC error to
>> signal the user exhausted the DMA mappings available for a container.
>>
>> The block driver started to mis-behave:
>>
>> qemu-system-x86_64: VFIO_MAP_DMA failed: No space left on device
>> (qemu)
>> (qemu) info status
>> VM status: paused (io-error)
>> (qemu) c
>> VFIO_MAP_DMA failed: No space left on device
>> qemu-system-x86_64: block/block-backend.c:1968: blk_get_aio_context: Assertion `ctx == blk->ctx' failed.
>
> Hi Phil,
>
>
> The diff looks good to me, but I’m not sure what exactly caused the assertion failure. There is `if (r) { goto fail; }` that handles -ENOSPC before, so it should be treated as a general case. What am I missing?
Good catch, ENOSPC ends setting BLOCK_DEVICE_IO_STATUS_NOSPACE
-> BLOCK_ERROR_ACTION_STOP, so the VM is paused with DMA mapping
exhausted. I don't understand the full "VM resume" path, but this
is not what we want (IO_NOSPACE is to warn the operator to add
more storage and resume, which is pointless in our case, resuming
won't help until we flush the mappings).
IIUC what we want is return ENOMEM to set BLOCK_DEVICE_IO_STATUS_FAILED.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device
2021-06-21 15:13 ` Philippe Mathieu-Daudé
@ 2021-06-21 15:36 ` Fam Zheng
2021-06-22 7:29 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2021-06-21 15:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, qemu-block, qemu-devel, Michal Prívozník,
qemu-stable, Maxim Levitsky, Eric Auger, Alex Williamson,
Stefan Hajnoczi, Max Reitz
> On 21 Jun 2021, at 16:13, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 6/21/21 3:18 PM, Fam Zheng wrote:
>>
>>
>>> On 21 Jun 2021, at 10:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>> When the NVMe block driver was introduced (see commit bdd6a90a9e5,
>>> January 2018), Linux VFIO_IOMMU_MAP_DMA ioctl was only returning
>>> -ENOMEM in case of error. The driver was correctly handling the
>>> error path to recycle its volatile IOVA mappings.
>>>
>>> To fix CVE-2019-3882, Linux commit 492855939bdb ("vfio/type1: Limit
>>> DMA mappings per container", April 2019) added the -ENOSPC error to
>>> signal the user exhausted the DMA mappings available for a container.
>>>
>>> The block driver started to mis-behave:
>>>
>>> qemu-system-x86_64: VFIO_MAP_DMA failed: No space left on device
>>> (qemu)
>>> (qemu) info status
>>> VM status: paused (io-error)
>>> (qemu) c
>>> VFIO_MAP_DMA failed: No space left on device
>>> qemu-system-x86_64: block/block-backend.c:1968: blk_get_aio_context: Assertion `ctx == blk->ctx' failed.
>>
>> Hi Phil,
>>
>>
>> The diff looks good to me, but I’m not sure what exactly caused the assertion failure. There is `if (r) { goto fail; }` that handles -ENOSPC before, so it should be treated as a general case. What am I missing?
>
> Good catch, ENOSPC ends setting BLOCK_DEVICE_IO_STATUS_NOSPACE
> -> BLOCK_ERROR_ACTION_STOP, so the VM is paused with DMA mapping
> exhausted. I don't understand the full "VM resume" path, but this
> is not what we want (IO_NOSPACE is to warn the operator to add
> more storage and resume, which is pointless in our case, resuming
> won't help until we flush the mappings).
>
> IIUC what we want is return ENOMEM to set BLOCK_DEVICE_IO_STATUS_FAILED.
I agree with that. It just makes me feel there’s another bug in the resuming code path. Can you get a backtrace?
Fam
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device
2021-06-21 15:36 ` Fam Zheng
@ 2021-06-22 7:29 ` Philippe Mathieu-Daudé
2021-06-22 8:06 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-22 7:29 UTC (permalink / raw)
To: Fam Zheng
Cc: Kevin Wolf, qemu-block, qemu-devel, Michal Prívozník,
qemu-stable, Maxim Levitsky, Eric Auger, Alex Williamson,
Stefan Hajnoczi, Max Reitz
On 6/21/21 5:36 PM, Fam Zheng wrote:
>> On 21 Jun 2021, at 16:13, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> On 6/21/21 3:18 PM, Fam Zheng wrote:
>>>> On 21 Jun 2021, at 10:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>>
>>>> When the NVMe block driver was introduced (see commit bdd6a90a9e5,
>>>> January 2018), Linux VFIO_IOMMU_MAP_DMA ioctl was only returning
>>>> -ENOMEM in case of error. The driver was correctly handling the
>>>> error path to recycle its volatile IOVA mappings.
>>>>
>>>> To fix CVE-2019-3882, Linux commit 492855939bdb ("vfio/type1: Limit
>>>> DMA mappings per container", April 2019) added the -ENOSPC error to
>>>> signal the user exhausted the DMA mappings available for a container.
>>>>
>>>> The block driver started to mis-behave:
>>>>
>>>> qemu-system-x86_64: VFIO_MAP_DMA failed: No space left on device
>>>> (qemu)
>>>> (qemu) info status
>>>> VM status: paused (io-error)
>>>> (qemu) c
>>>> VFIO_MAP_DMA failed: No space left on device
>>>> qemu-system-x86_64: block/block-backend.c:1968: blk_get_aio_context: Assertion `ctx == blk->ctx' failed.
>>>
>>> Hi Phil,
>>>
>>>
>>> The diff looks good to me, but I’m not sure what exactly caused the assertion failure. There is `if (r) { goto fail; }` that handles -ENOSPC before, so it should be treated as a general case. What am I missing?
>>
>> Good catch, ENOSPC ends setting BLOCK_DEVICE_IO_STATUS_NOSPACE
>> -> BLOCK_ERROR_ACTION_STOP, so the VM is paused with DMA mapping
>> exhausted. I don't understand the full "VM resume" path, but this
>> is not what we want (IO_NOSPACE is to warn the operator to add
>> more storage and resume, which is pointless in our case, resuming
>> won't help until we flush the mappings).
>>
>> IIUC what we want is return ENOMEM to set BLOCK_DEVICE_IO_STATUS_FAILED.
>
> I agree with that. It just makes me feel there’s another bug in the resuming code path. Can you get a backtrace?
It seems the resuming code path bug has been fixed elsewhere:
(qemu) info status
info status
VM status: paused (io-error)
(qemu) c
c
2021-06-22T07:27:00.745466Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
space left on device
(qemu) info status
info status
VM status: paused (io-error)
(qemu) c
c
2021-06-22T07:27:12.458137Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
space left on device
(qemu) c
c
2021-06-22T07:27:13.439167Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
space left on device
(qemu) c
c
2021-06-22T07:27:14.272071Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
space left on device
(qemu)
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device
2021-06-22 7:29 ` Philippe Mathieu-Daudé
@ 2021-06-22 8:06 ` Philippe Mathieu-Daudé
2021-06-22 12:42 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-22 8:06 UTC (permalink / raw)
To: Fam Zheng
Cc: Kevin Wolf, qemu-block, qemu-devel, Michal Prívozník,
qemu-stable, Maxim Levitsky, Eric Auger, Alex Williamson,
Stefan Hajnoczi, Max Reitz
On 6/22/21 9:29 AM, Philippe Mathieu-Daudé wrote:
> On 6/21/21 5:36 PM, Fam Zheng wrote:
>>> On 21 Jun 2021, at 16:13, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>> On 6/21/21 3:18 PM, Fam Zheng wrote:
>>>>> On 21 Jun 2021, at 10:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>>>
>>>>> When the NVMe block driver was introduced (see commit bdd6a90a9e5,
>>>>> January 2018), Linux VFIO_IOMMU_MAP_DMA ioctl was only returning
>>>>> -ENOMEM in case of error. The driver was correctly handling the
>>>>> error path to recycle its volatile IOVA mappings.
>>>>>
>>>>> To fix CVE-2019-3882, Linux commit 492855939bdb ("vfio/type1: Limit
>>>>> DMA mappings per container", April 2019) added the -ENOSPC error to
>>>>> signal the user exhausted the DMA mappings available for a container.
>>>>>
>>>>> The block driver started to mis-behave:
>>>>>
>>>>> qemu-system-x86_64: VFIO_MAP_DMA failed: No space left on device
>>>>> (qemu)
>>>>> (qemu) info status
>>>>> VM status: paused (io-error)
>>>>> (qemu) c
>>>>> VFIO_MAP_DMA failed: No space left on device
>>>>> qemu-system-x86_64: block/block-backend.c:1968: blk_get_aio_context: Assertion `ctx == blk->ctx' failed.
>>>>
>>>> Hi Phil,
>>>>
>>>>
>>>> The diff looks good to me, but I’m not sure what exactly caused the assertion failure. There is `if (r) { goto fail; }` that handles -ENOSPC before, so it should be treated as a general case. What am I missing?
>>>
>>> Good catch, ENOSPC ends setting BLOCK_DEVICE_IO_STATUS_NOSPACE
>>> -> BLOCK_ERROR_ACTION_STOP, so the VM is paused with DMA mapping
>>> exhausted. I don't understand the full "VM resume" path, but this
>>> is not what we want (IO_NOSPACE is to warn the operator to add
>>> more storage and resume, which is pointless in our case, resuming
>>> won't help until we flush the mappings).
>>>
>>> IIUC what we want is return ENOMEM to set BLOCK_DEVICE_IO_STATUS_FAILED.
>>
>> I agree with that. It just makes me feel there’s another bug in the resuming code path. Can you get a backtrace?
>
> It seems the resuming code path bug has been fixed elsewhere:
>
> (qemu) info status
> info status
> VM status: paused (io-error)
> (qemu) c
> c
> 2021-06-22T07:27:00.745466Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
> space left on device
> (qemu) info status
> info status
> VM status: paused (io-error)
> (qemu) c
> c
> 2021-06-22T07:27:12.458137Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
> space left on device
> (qemu) c
> c
> 2021-06-22T07:27:13.439167Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
> space left on device
> (qemu) c
> c
> 2021-06-22T07:27:14.272071Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
> space left on device
> (qemu)
>
I tested all releases up to v4.1.0 and could not trigger the
blk_get_aio_context() assertion. Building using --enable-debug.
IIRC Gentoo is more aggressive, so I'll restart using -O2.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device
2021-06-22 8:06 ` Philippe Mathieu-Daudé
@ 2021-06-22 12:42 ` Philippe Mathieu-Daudé
2021-06-22 13:12 ` Fam Zheng
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-06-22 12:42 UTC (permalink / raw)
To: Fam Zheng
Cc: Kevin Wolf, qemu-block, qemu-devel, Michal Prívozník,
qemu-stable, Maxim Levitsky, Eric Auger, Alex Williamson,
Stefan Hajnoczi, Max Reitz
On 6/22/21 10:06 AM, Philippe Mathieu-Daudé wrote:
> On 6/22/21 9:29 AM, Philippe Mathieu-Daudé wrote:
>> On 6/21/21 5:36 PM, Fam Zheng wrote:
>>>> On 21 Jun 2021, at 16:13, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>> On 6/21/21 3:18 PM, Fam Zheng wrote:
>>>>>> On 21 Jun 2021, at 10:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>>>>
>>>>>> When the NVMe block driver was introduced (see commit bdd6a90a9e5,
>>>>>> January 2018), Linux VFIO_IOMMU_MAP_DMA ioctl was only returning
>>>>>> -ENOMEM in case of error. The driver was correctly handling the
>>>>>> error path to recycle its volatile IOVA mappings.
>>>>>>
>>>>>> To fix CVE-2019-3882, Linux commit 492855939bdb ("vfio/type1: Limit
>>>>>> DMA mappings per container", April 2019) added the -ENOSPC error to
>>>>>> signal the user exhausted the DMA mappings available for a container.
>>>>>>
>>>>>> The block driver started to mis-behave:
>>>>>>
>>>>>> qemu-system-x86_64: VFIO_MAP_DMA failed: No space left on device
>>>>>> (qemu)
>>>>>> (qemu) info status
>>>>>> VM status: paused (io-error)
>>>>>> (qemu) c
>>>>>> VFIO_MAP_DMA failed: No space left on device
>>>>>> qemu-system-x86_64: block/block-backend.c:1968: blk_get_aio_context: Assertion `ctx == blk->ctx' failed.
>>>>>
>>>>> Hi Phil,
>>>>>
>>>>>
>>>>> The diff looks good to me, but I’m not sure what exactly caused the assertion failure. There is `if (r) { goto fail; }` that handles -ENOSPC before, so it should be treated as a general case. What am I missing?
>>>>
>>>> Good catch, ENOSPC ends setting BLOCK_DEVICE_IO_STATUS_NOSPACE
>>>> -> BLOCK_ERROR_ACTION_STOP, so the VM is paused with DMA mapping
>>>> exhausted. I don't understand the full "VM resume" path, but this
>>>> is not what we want (IO_NOSPACE is to warn the operator to add
>>>> more storage and resume, which is pointless in our case, resuming
>>>> won't help until we flush the mappings).
>>>>
>>>> IIUC what we want is return ENOMEM to set BLOCK_DEVICE_IO_STATUS_FAILED.
>>>
>>> I agree with that. It just makes me feel there’s another bug in the resuming code path. Can you get a backtrace?
>>
>> It seems the resuming code path bug has been fixed elsewhere:
>>
>> (qemu) info status
>> info status
>> VM status: paused (io-error)
>> (qemu) c
>> c
>> 2021-06-22T07:27:00.745466Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
>> space left on device
>> (qemu) info status
>> info status
>> VM status: paused (io-error)
>> (qemu) c
>> c
>> 2021-06-22T07:27:12.458137Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
>> space left on device
>> (qemu) c
>> c
>> 2021-06-22T07:27:13.439167Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
>> space left on device
>> (qemu) c
>> c
>> 2021-06-22T07:27:14.272071Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
>> space left on device
>> (qemu)
>>
>
> I tested all releases up to v4.1.0 and could not trigger the
> blk_get_aio_context() assertion. Building using --enable-debug.
> IIRC Gentoo is more aggressive, so I'll restart using -O2.
Took 4h30 to test all releases with -O3, couldn't reproduce :(
I wish I hadn't postponed writing an Ansible test script...
On v1 Michal said he doesn't have access to the machine anymore,
so I'll assume the other issue got fixed elsewhere.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device
2021-06-22 12:42 ` Philippe Mathieu-Daudé
@ 2021-06-22 13:12 ` Fam Zheng
0 siblings, 0 replies; 8+ messages in thread
From: Fam Zheng @ 2021-06-22 13:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, qemu-block, Michal Prívozník, qemu-stable,
qemu-devel, Maxim Levitsky, Eric Auger, Alex Williamson,
Stefan Hajnoczi, Max Reitz
> On 22 Jun 2021, at 13:42, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 6/22/21 10:06 AM, Philippe Mathieu-Daudé wrote:
>> On 6/22/21 9:29 AM, Philippe Mathieu-Daudé wrote:
>>> On 6/21/21 5:36 PM, Fam Zheng wrote:
>>>>> On 21 Jun 2021, at 16:13, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>>> On 6/21/21 3:18 PM, Fam Zheng wrote:
>>>>>>> On 21 Jun 2021, at 10:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>>>>>
>>>>>>> When the NVMe block driver was introduced (see commit bdd6a90a9e5,
>>>>>>> January 2018), Linux VFIO_IOMMU_MAP_DMA ioctl was only returning
>>>>>>> -ENOMEM in case of error. The driver was correctly handling the
>>>>>>> error path to recycle its volatile IOVA mappings.
>>>>>>>
>>>>>>> To fix CVE-2019-3882, Linux commit 492855939bdb ("vfio/type1: Limit
>>>>>>> DMA mappings per container", April 2019) added the -ENOSPC error to
>>>>>>> signal the user exhausted the DMA mappings available for a container.
>>>>>>>
>>>>>>> The block driver started to mis-behave:
>>>>>>>
>>>>>>> qemu-system-x86_64: VFIO_MAP_DMA failed: No space left on device
>>>>>>> (qemu)
>>>>>>> (qemu) info status
>>>>>>> VM status: paused (io-error)
>>>>>>> (qemu) c
>>>>>>> VFIO_MAP_DMA failed: No space left on device
>>>>>>> qemu-system-x86_64: block/block-backend.c:1968: blk_get_aio_context: Assertion `ctx == blk->ctx' failed.
>>>>>>
>>>>>> Hi Phil,
>>>>>>
>>>>>>
>>>>>> The diff looks good to me, but I’m not sure what exactly caused the assertion failure. There is `if (r) { goto fail; }` that handles -ENOSPC before, so it should be treated as a general case. What am I missing?
>>>>>
>>>>> Good catch, ENOSPC ends setting BLOCK_DEVICE_IO_STATUS_NOSPACE
>>>>> -> BLOCK_ERROR_ACTION_STOP, so the VM is paused with DMA mapping
>>>>> exhausted. I don't understand the full "VM resume" path, but this
>>>>> is not what we want (IO_NOSPACE is to warn the operator to add
>>>>> more storage and resume, which is pointless in our case, resuming
>>>>> won't help until we flush the mappings).
>>>>>
>>>>> IIUC what we want is return ENOMEM to set BLOCK_DEVICE_IO_STATUS_FAILED.
>>>>
>>>> I agree with that. It just makes me feel there’s another bug in the resuming code path. Can you get a backtrace?
>>>
>>> It seems the resuming code path bug has been fixed elsewhere:
>>>
>>> (qemu) info status
>>> info status
>>> VM status: paused (io-error)
>>> (qemu) c
>>> c
>>> 2021-06-22T07:27:00.745466Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
>>> space left on device
>>> (qemu) info status
>>> info status
>>> VM status: paused (io-error)
>>> (qemu) c
>>> c
>>> 2021-06-22T07:27:12.458137Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
>>> space left on device
>>> (qemu) c
>>> c
>>> 2021-06-22T07:27:13.439167Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
>>> space left on device
>>> (qemu) c
>>> c
>>> 2021-06-22T07:27:14.272071Z qemu-system-x86_64: VFIO_MAP_DMA failed: No
>>> space left on device
>>> (qemu)
>>>
>>
>> I tested all releases up to v4.1.0 and could not trigger the
>> blk_get_aio_context() assertion. Building using --enable-debug.
>> IIRC Gentoo is more aggressive, so I'll restart using -O2.
>
> Took 4h30 to test all releases with -O3, couldn't reproduce :(
>
> I wish I hadn't postponed writing an Ansible test script...
>
> On v1 Michal said he doesn't have access to the machine anymore,
> so I'll assume the other issue got fixed elsewhere.
>
>
Cool, then I think it’s just the commit message that should be updated.
Fam
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-22 13:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-21 9:32 [PATCH v2] block/nvme: Fix VFIO_MAP_DMA failed: No space left on device Philippe Mathieu-Daudé
2021-06-21 13:18 ` Fam Zheng
2021-06-21 15:13 ` Philippe Mathieu-Daudé
2021-06-21 15:36 ` Fam Zheng
2021-06-22 7:29 ` Philippe Mathieu-Daudé
2021-06-22 8:06 ` Philippe Mathieu-Daudé
2021-06-22 12:42 ` Philippe Mathieu-Daudé
2021-06-22 13:12 ` Fam Zheng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.