* [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions
@ 2020-04-17 7:44 Yan Zhao
2020-04-17 8:48 ` Philippe Mathieu-Daudé
2020-04-25 10:55 ` Paolo Bonzini
0 siblings, 2 replies; 7+ messages in thread
From: Yan Zhao @ 2020-04-17 7:44 UTC (permalink / raw)
To: pbonzini, alex.williamson; +Cc: Yan Zhao, xin.zeng, philmd, qemu-devel
for ram device regions, drop guest writes if the regions is read-only.
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
Signed-off-by: Xin Zeng <xin.zeng@intel.com>
---
memory.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/memory.c b/memory.c
index 601b749906..9576dd6807 100644
--- a/memory.c
+++ b/memory.c
@@ -34,6 +34,7 @@
#include "sysemu/accel.h"
#include "hw/boards.h"
#include "migration/vmstate.h"
+#include "qemu/log.h"
//#define DEBUG_UNASSIGNED
@@ -1313,6 +1314,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
MemoryRegion *mr = opaque;
trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
+ if (mr->readonly) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid write to read only ram device region 0x%"
+ HWADDR_PRIx" size %u\n", addr, size);
+ return;
+ }
switch (size) {
case 1:
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions
2020-04-17 7:44 [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions Yan Zhao
@ 2020-04-17 8:48 ` Philippe Mathieu-Daudé
2020-04-25 10:55 ` Paolo Bonzini
1 sibling, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-17 8:48 UTC (permalink / raw)
To: Yan Zhao, pbonzini, alex.williamson; +Cc: xin.zeng, qemu-devel
On 4/17/20 9:44 AM, Yan Zhao wrote:
> for ram device regions, drop guest writes if the regions is read-only.
>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> ---
> memory.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/memory.c b/memory.c
> index 601b749906..9576dd6807 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -34,6 +34,7 @@
> #include "sysemu/accel.h"
> #include "hw/boards.h"
> #include "migration/vmstate.h"
> +#include "qemu/log.h"
>
> //#define DEBUG_UNASSIGNED
>
> @@ -1313,6 +1314,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> MemoryRegion *mr = opaque;
>
> trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> + if (mr->readonly) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "Invalid write to read only ram device region 0x%"
> + HWADDR_PRIx" size %u\n", addr, size);
> + return;
> + }
>
> switch (size) {
> case 1:
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions
2020-04-17 7:44 [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions Yan Zhao
2020-04-17 8:48 ` Philippe Mathieu-Daudé
@ 2020-04-25 10:55 ` Paolo Bonzini
2020-04-26 1:04 ` Yan Zhao
1 sibling, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2020-04-25 10:55 UTC (permalink / raw)
To: Yan Zhao, alex.williamson; +Cc: xin.zeng, philmd, qemu-devel
On 17/04/20 09:44, Yan Zhao wrote:
> for ram device regions, drop guest writes if the regions is read-only.
>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> ---
> memory.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/memory.c b/memory.c
> index 601b749906..9576dd6807 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -34,6 +34,7 @@
> #include "sysemu/accel.h"
> #include "hw/boards.h"
> #include "migration/vmstate.h"
> +#include "qemu/log.h"
>
> //#define DEBUG_UNASSIGNED
>
> @@ -1313,6 +1314,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> MemoryRegion *mr = opaque;
>
> trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> + if (mr->readonly) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "Invalid write to read only ram device region 0x%"
> + HWADDR_PRIx" size %u\n", addr, size);
> + return;
> + }
As mentioned in the review of v1, memory_region_ram_device_write should
be changed to a .write_with_attrs operation, so that it can return
MEMTX_ERROR.
Otherwise this looks good.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions
2020-04-25 10:55 ` Paolo Bonzini
@ 2020-04-26 1:04 ` Yan Zhao
2020-04-27 9:15 ` Yan Zhao
0 siblings, 1 reply; 7+ messages in thread
From: Yan Zhao @ 2020-04-26 1:04 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Zeng, Xin, alex.williamson, philmd, qemu-devel
On Sat, Apr 25, 2020 at 06:55:33PM +0800, Paolo Bonzini wrote:
> On 17/04/20 09:44, Yan Zhao wrote:
> > for ram device regions, drop guest writes if the regions is read-only.
> >
> > Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> > ---
> > memory.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/memory.c b/memory.c
> > index 601b749906..9576dd6807 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -34,6 +34,7 @@
> > #include "sysemu/accel.h"
> > #include "hw/boards.h"
> > #include "migration/vmstate.h"
> > +#include "qemu/log.h"
> >
> > //#define DEBUG_UNASSIGNED
> >
> > @@ -1313,6 +1314,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> > MemoryRegion *mr = opaque;
> >
> > trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> > + if (mr->readonly) {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "Invalid write to read only ram device region 0x%"
> > + HWADDR_PRIx" size %u\n", addr, size);
> > + return;
> > + }
>
> As mentioned in the review of v1, memory_region_ram_device_write should
> be changed to a .write_with_attrs operation, so that it can return
> MEMTX_ERROR.
>
> Otherwise this looks good.
>
hi Paolo,
thanks for pointing it out again!
I didn't get your meaning in v1. will update the patch!
Thanks
Yan
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions
2020-04-26 1:04 ` Yan Zhao
@ 2020-04-27 9:15 ` Yan Zhao
2020-04-27 9:31 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 7+ messages in thread
From: Yan Zhao @ 2020-04-27 9:15 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: philmd, alex.williamson, Zeng, Xin, qemu-devel
On Sun, Apr 26, 2020 at 09:04:31AM +0800, Yan Zhao wrote:
> On Sat, Apr 25, 2020 at 06:55:33PM +0800, Paolo Bonzini wrote:
> > On 17/04/20 09:44, Yan Zhao wrote:
> > > for ram device regions, drop guest writes if the regions is read-only.
> > >
> > > Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > > Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> > > ---
> > > memory.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/memory.c b/memory.c
> > > index 601b749906..9576dd6807 100644
> > > --- a/memory.c
> > > +++ b/memory.c
> > > @@ -34,6 +34,7 @@
> > > #include "sysemu/accel.h"
> > > #include "hw/boards.h"
> > > #include "migration/vmstate.h"
> > > +#include "qemu/log.h"
> > >
> > > //#define DEBUG_UNASSIGNED
> > >
> > > @@ -1313,6 +1314,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> > > MemoryRegion *mr = opaque;
> > >
> > > trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> > > + if (mr->readonly) {
> > > + qemu_log_mask(LOG_GUEST_ERROR,
> > > + "Invalid write to read only ram device region 0x%"
> > > + HWADDR_PRIx" size %u\n", addr, size);
> > > + return;
> > > + }
> >
> > As mentioned in the review of v1, memory_region_ram_device_write should
> > be changed to a .write_with_attrs operation, so that it can return
> > MEMTX_ERROR.
> >
hi Paolo and Alex,
need I also change vfio_region_write() in patch 2 to a .write_with_attrs
operation?
vfio_region_read() is also possible to fail, so should I change it to a
.read_with_attrs, too?
Thanks
Yan
> > Otherwise this looks good.
> >
> hi Paolo,
> thanks for pointing it out again!
> I didn't get your meaning in v1. will update the patch!
>
> Thanks
> Yan
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions
2020-04-27 9:15 ` Yan Zhao
@ 2020-04-27 9:31 ` Philippe Mathieu-Daudé
2020-04-27 10:09 ` Yan Zhao
0 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-27 9:31 UTC (permalink / raw)
To: Yan Zhao, Paolo Bonzini; +Cc: alex.williamson, Zeng, Xin, qemu-devel
On 4/27/20 11:15 AM, Yan Zhao wrote:
> On Sun, Apr 26, 2020 at 09:04:31AM +0800, Yan Zhao wrote:
>> On Sat, Apr 25, 2020 at 06:55:33PM +0800, Paolo Bonzini wrote:
>>> On 17/04/20 09:44, Yan Zhao wrote:
>>>> for ram device regions, drop guest writes if the regions is read-only.
>>>>
>>>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
>>>> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
>>>> ---
>>>> memory.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/memory.c b/memory.c
>>>> index 601b749906..9576dd6807 100644
>>>> --- a/memory.c
>>>> +++ b/memory.c
>>>> @@ -34,6 +34,7 @@
>>>> #include "sysemu/accel.h"
>>>> #include "hw/boards.h"
>>>> #include "migration/vmstate.h"
>>>> +#include "qemu/log.h"
>>>>
>>>> //#define DEBUG_UNASSIGNED
>>>>
>>>> @@ -1313,6 +1314,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
>>>> MemoryRegion *mr = opaque;
>>>>
>>>> trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
>>>> + if (mr->readonly) {
>>>> + qemu_log_mask(LOG_GUEST_ERROR,
>>>> + "Invalid write to read only ram device region 0x%"
>>>> + HWADDR_PRIx" size %u\n", addr, size);
>>>> + return;
>>>> + }
>>>
>>> As mentioned in the review of v1, memory_region_ram_device_write should
>>> be changed to a .write_with_attrs operation, so that it can return
>>> MEMTX_ERROR.
>>>
> hi Paolo and Alex,
> need I also change vfio_region_write() in patch 2 to a .write_with_attrs
> operation?
> vfio_region_read() is also possible to fail, so should I change it to a
> .read_with_attrs, too?
Yes.
Please submit your series as a thread, with a cover letter:
https://wiki.qemu.org/Contribute/SubmitAPatch#Include_a_meaningful_cover_letter
>
> Thanks
> Yan
>
>>> Otherwise this looks good.
>>>
>> hi Paolo,
>> thanks for pointing it out again!
>> I didn't get your meaning in v1. will update the patch!
>>
>> Thanks
>> Yan
>>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions
2020-04-27 9:31 ` Philippe Mathieu-Daudé
@ 2020-04-27 10:09 ` Yan Zhao
0 siblings, 0 replies; 7+ messages in thread
From: Yan Zhao @ 2020-04-27 10:09 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, alex.williamson, Zeng, Xin, qemu-devel
On Mon, Apr 27, 2020 at 05:31:48PM +0800, Philippe Mathieu-Daudé wrote:
> On 4/27/20 11:15 AM, Yan Zhao wrote:
> > On Sun, Apr 26, 2020 at 09:04:31AM +0800, Yan Zhao wrote:
> >> On Sat, Apr 25, 2020 at 06:55:33PM +0800, Paolo Bonzini wrote:
> >>> On 17/04/20 09:44, Yan Zhao wrote:
> >>>> for ram device regions, drop guest writes if the regions is read-only.
> >>>>
> >>>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> >>>> Signed-off-by: Xin Zeng <xin.zeng@intel.com>
> >>>> ---
> >>>> memory.c | 7 +++++++
> >>>> 1 file changed, 7 insertions(+)
> >>>>
> >>>> diff --git a/memory.c b/memory.c
> >>>> index 601b749906..9576dd6807 100644
> >>>> --- a/memory.c
> >>>> +++ b/memory.c
> >>>> @@ -34,6 +34,7 @@
> >>>> #include "sysemu/accel.h"
> >>>> #include "hw/boards.h"
> >>>> #include "migration/vmstate.h"
> >>>> +#include "qemu/log.h"
> >>>>
> >>>> //#define DEBUG_UNASSIGNED
> >>>>
> >>>> @@ -1313,6 +1314,12 @@ static void memory_region_ram_device_write(void *opaque, hwaddr addr,
> >>>> MemoryRegion *mr = opaque;
> >>>>
> >>>> trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data, size);
> >>>> + if (mr->readonly) {
> >>>> + qemu_log_mask(LOG_GUEST_ERROR,
> >>>> + "Invalid write to read only ram device region 0x%"
> >>>> + HWADDR_PRIx" size %u\n", addr, size);
> >>>> + return;
> >>>> + }
> >>>
> >>> As mentioned in the review of v1, memory_region_ram_device_write should
> >>> be changed to a .write_with_attrs operation, so that it can return
> >>> MEMTX_ERROR.
> >>>
> > hi Paolo and Alex,
> > need I also change vfio_region_write() in patch 2 to a .write_with_attrs
> > operation?
> > vfio_region_read() is also possible to fail, so should I change it to a
> > .read_with_attrs, too?
>
> Yes.
>
> Please submit your series as a thread, with a cover letter:
> https://wiki.qemu.org/Contribute/SubmitAPatch#Include_a_meaningful_cover_letter
>
hi Philippe
thanks for pointing out this issue.
I just realized this version of patches were sent separately, though I did send
a cover letter. not sure what happened. maybe I just forgot to add an
-in-reply-to before sending out.
will pay attention to it next time.
Thanks
Yan
> >
> > Thanks
> > Yan
> >
> >>> Otherwise this looks good.
> >>>
> >> hi Paolo,
> >> thanks for pointing it out again!
> >> I didn't get your meaning in v1. will update the patch!
> >>
> >> Thanks
> >> Yan
> >>>
> >>
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-27 10:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-17 7:44 [PATCH v4 1/3] memory: drop guest writes to read-only ram device regions Yan Zhao
2020-04-17 8:48 ` Philippe Mathieu-Daudé
2020-04-25 10:55 ` Paolo Bonzini
2020-04-26 1:04 ` Yan Zhao
2020-04-27 9:15 ` Yan Zhao
2020-04-27 9:31 ` Philippe Mathieu-Daudé
2020-04-27 10:09 ` Yan Zhao
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.