* [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal()
@ 2021-08-05 9:23 David Hildenbrand
2021-08-06 5:04 ` Pankaj Gupta
2021-08-16 20:52 ` Peter Xu
0 siblings, 2 replies; 6+ messages in thread
From: David Hildenbrand @ 2021-08-05 9:23 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Philippe Mathieu-Daudé, Peter Xu, David Hildenbrand
When adding RAM_NORESERVE, we forgot to remove the old assertion when
adding the updated one, most probably when reworking the patches or
rebasing. We can easily crash QEMU by adding
-object memory-backend-ram,id=mem0,size=500G,reserve=off
to the QEMU cmdline:
qemu-system-x86_64: ../softmmu/physmem.c:2146: qemu_ram_alloc_internal:
Assertion `(ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC))
== 0' failed.
Fix it by removing the old assertion.
Fixes: 8dbe22c6868b ("memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()")
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
v1 -> v2:
- Added rbs
- Tagged for 6.1 inclusion
---
softmmu/physmem.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 3c1912a1a0..2e18947598 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2143,7 +2143,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
RAMBlock *new_block;
Error *local_err = NULL;
- assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC)) == 0);
assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
RAM_NORESERVE)) == 0);
assert(!host ^ (ram_flags & RAM_PREALLOC));
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal()
2021-08-05 9:23 [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal() David Hildenbrand
@ 2021-08-06 5:04 ` Pankaj Gupta
2021-08-16 20:52 ` Peter Xu
1 sibling, 0 replies; 6+ messages in thread
From: Pankaj Gupta @ 2021-08-06 5:04 UTC (permalink / raw)
To: David Hildenbrand
Cc: Paolo Bonzini, Philippe Mathieu-Daudé, Qemu Developers, Peter Xu
> When adding RAM_NORESERVE, we forgot to remove the old assertion when
> adding the updated one, most probably when reworking the patches or
> rebasing. We can easily crash QEMU by adding
> -object memory-backend-ram,id=mem0,size=500G,reserve=off
> to the QEMU cmdline:
> qemu-system-x86_64: ../softmmu/physmem.c:2146: qemu_ram_alloc_internal:
> Assertion `(ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC))
> == 0' failed.
>
> Fix it by removing the old assertion.
>
> Fixes: 8dbe22c6868b ("memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()")
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>
> v1 -> v2:
> - Added rbs
> - Tagged for 6.1 inclusion
>
> ---
> softmmu/physmem.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 3c1912a1a0..2e18947598 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2143,7 +2143,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
> RAMBlock *new_block;
> Error *local_err = NULL;
>
> - assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC)) == 0);
> assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
> RAM_NORESERVE)) == 0);
> assert(!host ^ (ram_flags & RAM_PREALLOC));
> --
> 2.31.1
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal()
2021-08-05 9:23 [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal() David Hildenbrand
2021-08-06 5:04 ` Pankaj Gupta
@ 2021-08-16 20:52 ` Peter Xu
2021-08-17 7:14 ` David Hildenbrand
1 sibling, 1 reply; 6+ messages in thread
From: Peter Xu @ 2021-08-16 20:52 UTC (permalink / raw)
To: David Hildenbrand
Cc: Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, Peter Maydell
On Thu, Aug 05, 2021 at 11:23:50AM +0200, David Hildenbrand wrote:
> When adding RAM_NORESERVE, we forgot to remove the old assertion when
> adding the updated one, most probably when reworking the patches or
> rebasing. We can easily crash QEMU by adding
> -object memory-backend-ram,id=mem0,size=500G,reserve=off
> to the QEMU cmdline:
> qemu-system-x86_64: ../softmmu/physmem.c:2146: qemu_ram_alloc_internal:
> Assertion `(ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC))
> == 0' failed.
>
> Fix it by removing the old assertion.
>
> Fixes: 8dbe22c6868b ("memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()")
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>
> v1 -> v2:
> - Added rbs
> - Tagged for 6.1 inclusion
>
> ---
> softmmu/physmem.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 3c1912a1a0..2e18947598 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2143,7 +2143,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
> RAMBlock *new_block;
> Error *local_err = NULL;
>
> - assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC)) == 0);
> assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
> RAM_NORESERVE)) == 0);
> assert(!host ^ (ram_flags & RAM_PREALLOC));
> --
> 2.31.1
>
Today I just noticed this patch is still missing for 6.1. How many users are
there with reserve=off? Would it be worth rc4 or not?
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal()
2021-08-16 20:52 ` Peter Xu
@ 2021-08-17 7:14 ` David Hildenbrand
2021-08-17 14:25 ` Peter Xu
2021-08-17 15:51 ` Peter Maydell
0 siblings, 2 replies; 6+ messages in thread
From: David Hildenbrand @ 2021-08-17 7:14 UTC (permalink / raw)
To: Peter Xu
Cc: Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, Peter Maydell
On 16.08.21 22:52, Peter Xu wrote:
> On Thu, Aug 05, 2021 at 11:23:50AM +0200, David Hildenbrand wrote:
>> When adding RAM_NORESERVE, we forgot to remove the old assertion when
>> adding the updated one, most probably when reworking the patches or
>> rebasing. We can easily crash QEMU by adding
>> -object memory-backend-ram,id=mem0,size=500G,reserve=off
>> to the QEMU cmdline:
>> qemu-system-x86_64: ../softmmu/physmem.c:2146: qemu_ram_alloc_internal:
>> Assertion `(ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC))
>> == 0' failed.
>>
>> Fix it by removing the old assertion.
>>
>> Fixes: 8dbe22c6868b ("memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()")
>> Reviewed-by: Peter Xu <peterx@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>
>> v1 -> v2:
>> - Added rbs
>> - Tagged for 6.1 inclusion
>>
>> ---
>> softmmu/physmem.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
>> index 3c1912a1a0..2e18947598 100644
>> --- a/softmmu/physmem.c
>> +++ b/softmmu/physmem.c
>> @@ -2143,7 +2143,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
>> RAMBlock *new_block;
>> Error *local_err = NULL;
>>
>> - assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC)) == 0);
>> assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
>> RAM_NORESERVE)) == 0);
>> assert(!host ^ (ram_flags & RAM_PREALLOC));
>> --
>> 2.31.1
>>
>
> Today I just noticed this patch is still missing for 6.1. How many users are
> there with reserve=off? Would it be worth rc4 or not?
>
Indeed, I forgot to follow up, thanks for bringing this up.
Libvirt does not support virtio-mem yet and consequently doesn't support
reserve=off yet. (there are use cases without virtio-mem, but I don't
think anybody is using it yet)
It's an easy way to crash QEMU, but we could also fix in the -stable
tree instead.
(most probably you and me should also be doing PULL requests for "Memory
API", we'll have to discuss with Paolo)
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal()
2021-08-17 7:14 ` David Hildenbrand
@ 2021-08-17 14:25 ` Peter Xu
2021-08-17 15:51 ` Peter Maydell
1 sibling, 0 replies; 6+ messages in thread
From: Peter Xu @ 2021-08-17 14:25 UTC (permalink / raw)
To: David Hildenbrand
Cc: Paolo Bonzini, Philippe Mathieu-Daudé, qemu-devel, Peter Maydell
On Tue, Aug 17, 2021 at 09:14:32AM +0200, David Hildenbrand wrote:
> Libvirt does not support virtio-mem yet and consequently doesn't support
> reserve=off yet. (there are use cases without virtio-mem, but I don't think
> anybody is using it yet)
>
> It's an easy way to crash QEMU, but we could also fix in the -stable tree
> instead.
I hit this when trying with some extreme bitmap tests then I remembered this
patch, but that shouldn't really be used by any real customer for sure.
Sounds good then, thanks.
--
Peter Xu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal()
2021-08-17 7:14 ` David Hildenbrand
2021-08-17 14:25 ` Peter Xu
@ 2021-08-17 15:51 ` Peter Maydell
1 sibling, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2021-08-17 15:51 UTC (permalink / raw)
To: David Hildenbrand
Cc: Paolo Bonzini, Philippe Mathieu-Daudé, QEMU Developers, Peter Xu
On Tue, 17 Aug 2021 at 08:14, David Hildenbrand <david@redhat.com> wrote:
>
> On 16.08.21 22:52, Peter Xu wrote:
> > On Thu, Aug 05, 2021 at 11:23:50AM +0200, David Hildenbrand wrote:
> >> When adding RAM_NORESERVE, we forgot to remove the old assertion when
> >> adding the updated one, most probably when reworking the patches or
> >> rebasing. We can easily crash QEMU by adding
> >> -object memory-backend-ram,id=mem0,size=500G,reserve=off
> >> to the QEMU cmdline:
> >> qemu-system-x86_64: ../softmmu/physmem.c:2146: qemu_ram_alloc_internal:
> >> Assertion `(ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC))
> >> == 0' failed.
> >>
> >> Fix it by removing the old assertion.
> >>
> >> Fixes: 8dbe22c6868b ("memory: Introduce RAM_NORESERVE and wire it up in qemu_ram_mmap()")
> >> Reviewed-by: Peter Xu <peterx@redhat.com>
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >> Cc: Peter Xu <peterx@redhat.com>
> >> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >> ---
> >>
> >> v1 -> v2:
> >> - Added rbs
> >> - Tagged for 6.1 inclusion
> >>
> >> ---
> >> softmmu/physmem.c | 1 -
> >> 1 file changed, 1 deletion(-)
> >>
> >> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> >> index 3c1912a1a0..2e18947598 100644
> >> --- a/softmmu/physmem.c
> >> +++ b/softmmu/physmem.c
> >> @@ -2143,7 +2143,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size, ram_addr_t max_size,
> >> RAMBlock *new_block;
> >> Error *local_err = NULL;
> >>
> >> - assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC)) == 0);
> >> assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
> >> RAM_NORESERVE)) == 0);
> >> assert(!host ^ (ram_flags & RAM_PREALLOC));
> >> --
> >> 2.31.1
> >>
> >
> > Today I just noticed this patch is still missing for 6.1. How many users are
> > there with reserve=off? Would it be worth rc4 or not?
> >
>
> Indeed, I forgot to follow up, thanks for bringing this up.
>
> Libvirt does not support virtio-mem yet and consequently doesn't support
> reserve=off yet. (there are use cases without virtio-mem, but I don't
> think anybody is using it yet)
>
> It's an easy way to crash QEMU, but we could also fix in the -stable
> tree instead.
It is a really obvious right fix, though, so I'm going to apply
it for rc4 anyway.
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-17 15:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05 9:23 [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal() David Hildenbrand
2021-08-06 5:04 ` Pankaj Gupta
2021-08-16 20:52 ` Peter Xu
2021-08-17 7:14 ` David Hildenbrand
2021-08-17 14:25 ` Peter Xu
2021-08-17 15:51 ` Peter Maydell
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.