* [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@redhat.com, philmd@redhat.com, qemu-devel@nongnu.org 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@redhat.com, alex.williamson@redhat.com, Zeng, Xin, qemu-devel@nongnu.org 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@redhat.com, Zeng, Xin, qemu-devel@nongnu.org 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@redhat.com, Zeng, Xin, qemu-devel@nongnu.org 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).