qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] softmmu/physmem: Fix address of FlatView access in address_space_(read|write)_cached_slow()
@ 2022-09-05  0:00 Alberto Faria
  2022-09-05  7:45 ` Paolo Bonzini
  0 siblings, 1 reply; 4+ messages in thread
From: Alberto Faria @ 2022-09-05  0:00 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Peter Xu, Philippe Mathieu-Daudé,
	David Hildenbrand, Alberto Faria, Stefan Hajnoczi

flatview_(read|write)_continue() must be called with an address in the
FlatView's address space, but `addr` is relative to the
MemoryRegionCache.

Convert `addr` from the MemoryRegionCache's address space to the
FlatView's before passing it to flatview_(read|write)_continue().

This problem was uncovered while attempting to perform unaligned writes,
which caused flatview_write_continue() to reach the call to
flatview_translate(), which then translated the erroneous address and
caused the subsequent write to fail.

Fixes: 48564041a7 ("exec: reintroduce MemoryRegion caching")
Co-Developed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 softmmu/physmem.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 50231bab30..bdde4eb927 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3420,6 +3420,14 @@ static inline MemoryRegion *address_space_translate_cached(
     return section.mr;
 }
 
+/* Converts `addr` from the address space of `cache` to that of `cache->fv`. */
+static inline hwaddr addr_in_cache_to_in_flat_view(MemoryRegionCache *cache,
+                                                   hwaddr addr)
+{
+    hwaddr addr_in_mrs = addr + cache->xlat - cache->mrs.offset_within_region;
+    return addr_in_mrs + cache->mrs.offset_within_address_space;
+}
+
 /* Called from RCU critical section. address_space_read_cached uses this
  * out of line function when the target is an MMIO or IOMMU region.
  */
@@ -3434,8 +3442,9 @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr,
     mr = address_space_translate_cached(cache, addr, &addr1, &l, false,
                                         MEMTXATTRS_UNSPECIFIED);
     return flatview_read_continue(cache->fv,
-                                  addr, MEMTXATTRS_UNSPECIFIED, buf, len,
-                                  addr1, l, mr);
+                                  addr_in_cache_to_in_flat_view(cache, addr),
+                                  MEMTXATTRS_UNSPECIFIED, buf, len, addr1, l,
+                                  mr);
 }
 
 /* Called from RCU critical section. address_space_write_cached uses this
@@ -3452,8 +3461,9 @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr,
     mr = address_space_translate_cached(cache, addr, &addr1, &l, true,
                                         MEMTXATTRS_UNSPECIFIED);
     return flatview_write_continue(cache->fv,
-                                   addr, MEMTXATTRS_UNSPECIFIED, buf, len,
-                                   addr1, l, mr);
+                                   addr_in_cache_to_in_flat_view(cache, addr),
+                                   MEMTXATTRS_UNSPECIFIED, buf, len, addr1, l,
+                                   mr);
 }
 
 #define ARG1_DECL                MemoryRegionCache *cache
-- 
2.37.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] softmmu/physmem: Fix address of FlatView access in address_space_(read|write)_cached_slow()
  2022-09-05  0:00 [PATCH v2] softmmu/physmem: Fix address of FlatView access in address_space_(read|write)_cached_slow() Alberto Faria
@ 2022-09-05  7:45 ` Paolo Bonzini
  2022-12-02 12:29   ` Alberto Faria
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2022-09-05  7:45 UTC (permalink / raw)
  To: Alberto Faria
  Cc: qemu-devel, Peter Xu, Philippe Mathieu-Daudé,
	David Hildenbrand, Stefan Hajnoczi

[-- Attachment #1: Type: text/plain, Size: 3163 bytes --]

Thanks, I will queue it.

Paolo

Il lun 5 set 2022, 02:00 Alberto Faria <afaria@redhat.com> ha scritto:

> flatview_(read|write)_continue() must be called with an address in the
> FlatView's address space, but `addr` is relative to the
> MemoryRegionCache.
>
> Convert `addr` from the MemoryRegionCache's address space to the
> FlatView's before passing it to flatview_(read|write)_continue().
>
> This problem was uncovered while attempting to perform unaligned writes,
> which caused flatview_write_continue() to reach the call to
> flatview_translate(), which then translated the erroneous address and
> caused the subsequent write to fail.
>
> Fixes: 48564041a7 ("exec: reintroduce MemoryRegion caching")
> Co-Developed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>  softmmu/physmem.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 50231bab30..bdde4eb927 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -3420,6 +3420,14 @@ static inline MemoryRegion
> *address_space_translate_cached(
>      return section.mr;
>  }
>
> +/* Converts `addr` from the address space of `cache` to that of
> `cache->fv`. */
> +static inline hwaddr addr_in_cache_to_in_flat_view(MemoryRegionCache
> *cache,
> +                                                   hwaddr addr)
> +{
> +    hwaddr addr_in_mrs = addr + cache->xlat -
> cache->mrs.offset_within_region;
> +    return addr_in_mrs + cache->mrs.offset_within_address_space;
> +}
> +
>  /* Called from RCU critical section. address_space_read_cached uses this
>   * out of line function when the target is an MMIO or IOMMU region.
>   */
> @@ -3434,8 +3442,9 @@ address_space_read_cached_slow(MemoryRegionCache
> *cache, hwaddr addr,
>      mr = address_space_translate_cached(cache, addr, &addr1, &l, false,
>                                          MEMTXATTRS_UNSPECIFIED);
>      return flatview_read_continue(cache->fv,
> -                                  addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> -                                  addr1, l, mr);
> +                                  addr_in_cache_to_in_flat_view(cache,
> addr),
> +                                  MEMTXATTRS_UNSPECIFIED, buf, len,
> addr1, l,
> +                                  mr);
>  }
>
>  /* Called from RCU critical section. address_space_write_cached uses this
> @@ -3452,8 +3461,9 @@ address_space_write_cached_slow(MemoryRegionCache
> *cache, hwaddr addr,
>      mr = address_space_translate_cached(cache, addr, &addr1, &l, true,
>                                          MEMTXATTRS_UNSPECIFIED);
>      return flatview_write_continue(cache->fv,
> -                                   addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> -                                   addr1, l, mr);
> +                                   addr_in_cache_to_in_flat_view(cache,
> addr),
> +                                   MEMTXATTRS_UNSPECIFIED, buf, len,
> addr1, l,
> +                                   mr);
>  }
>
>  #define ARG1_DECL                MemoryRegionCache *cache
> --
> 2.37.2
>
>

[-- Attachment #2: Type: text/html, Size: 4220 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] softmmu/physmem: Fix address of FlatView access in address_space_(read|write)_cached_slow()
  2022-09-05  7:45 ` Paolo Bonzini
@ 2022-12-02 12:29   ` Alberto Faria
  2023-01-11 14:28     ` Stefan Hajnoczi
  0 siblings, 1 reply; 4+ messages in thread
From: Alberto Faria @ 2022-12-02 12:29 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: qemu-devel, Peter Xu, Philippe Mathieu-Daudé,
	David Hildenbrand, Stefan Hajnoczi

On Mon, Sep 5, 2022 at 8:45 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Thanks, I will queue it.
>
> Paolo
>
> Il lun 5 set 2022, 02:00 Alberto Faria <afaria@redhat.com> ha scritto:
>>
>> flatview_(read|write)_continue() must be called with an address in the
>> FlatView's address space, but `addr` is relative to the
>> MemoryRegionCache.
>>
>> Convert `addr` from the MemoryRegionCache's address space to the
>> FlatView's before passing it to flatview_(read|write)_continue().
>>
>> This problem was uncovered while attempting to perform unaligned writes,
>> which caused flatview_write_continue() to reach the call to
>> flatview_translate(), which then translated the erroneous address and
>> caused the subsequent write to fail.
>>
>> Fixes: 48564041a7 ("exec: reintroduce MemoryRegion caching")
>> Co-Developed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Alberto Faria <afaria@redhat.com>
>> ---
>>  softmmu/physmem.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
>> index 50231bab30..bdde4eb927 100644
>> --- a/softmmu/physmem.c
>> +++ b/softmmu/physmem.c
>> @@ -3420,6 +3420,14 @@ static inline MemoryRegion *address_space_translate_cached(
>>      return section.mr;
>>  }
>>
>> +/* Converts `addr` from the address space of `cache` to that of `cache->fv`. */
>> +static inline hwaddr addr_in_cache_to_in_flat_view(MemoryRegionCache *cache,
>> +                                                   hwaddr addr)
>> +{
>> +    hwaddr addr_in_mrs = addr + cache->xlat - cache->mrs.offset_within_region;
>> +    return addr_in_mrs + cache->mrs.offset_within_address_space;
>> +}
>> +
>>  /* Called from RCU critical section. address_space_read_cached uses this
>>   * out of line function when the target is an MMIO or IOMMU region.
>>   */
>> @@ -3434,8 +3442,9 @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr,
>>      mr = address_space_translate_cached(cache, addr, &addr1, &l, false,
>>                                          MEMTXATTRS_UNSPECIFIED);
>>      return flatview_read_continue(cache->fv,
>> -                                  addr, MEMTXATTRS_UNSPECIFIED, buf, len,
>> -                                  addr1, l, mr);
>> +                                  addr_in_cache_to_in_flat_view(cache, addr),
>> +                                  MEMTXATTRS_UNSPECIFIED, buf, len, addr1, l,
>> +                                  mr);
>>  }
>>
>>  /* Called from RCU critical section. address_space_write_cached uses this
>> @@ -3452,8 +3461,9 @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr,
>>      mr = address_space_translate_cached(cache, addr, &addr1, &l, true,
>>                                          MEMTXATTRS_UNSPECIFIED);
>>      return flatview_write_continue(cache->fv,
>> -                                   addr, MEMTXATTRS_UNSPECIFIED, buf, len,
>> -                                   addr1, l, mr);
>> +                                   addr_in_cache_to_in_flat_view(cache, addr),
>> +                                   MEMTXATTRS_UNSPECIFIED, buf, len, addr1, l,
>> +                                   mr);
>>  }
>>
>>  #define ARG1_DECL                MemoryRegionCache *cache
>> --
>> 2.37.2

This doesn't seem to be in master yet. Maybe it fell through the cracks?

Alberto



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] softmmu/physmem: Fix address of FlatView access in address_space_(read|write)_cached_slow()
  2022-12-02 12:29   ` Alberto Faria
@ 2023-01-11 14:28     ` Stefan Hajnoczi
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2023-01-11 14:28 UTC (permalink / raw)
  To: Alberto Faria, Paolo Bonzini
  Cc: qemu-devel, Peter Xu, Philippe Mathieu-Daudé,
	David Hildenbrand, Stefan Hajnoczi

On Fri, 2 Dec 2022 at 07:31, Alberto Faria <afaria@redhat.com> wrote:
>
> On Mon, Sep 5, 2022 at 8:45 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Thanks, I will queue it.
> >
> > Paolo
> >
> > Il lun 5 set 2022, 02:00 Alberto Faria <afaria@redhat.com> ha scritto:
> >>
> >> flatview_(read|write)_continue() must be called with an address in the
> >> FlatView's address space, but `addr` is relative to the
> >> MemoryRegionCache.
> >>
> >> Convert `addr` from the MemoryRegionCache's address space to the
> >> FlatView's before passing it to flatview_(read|write)_continue().
> >>
> >> This problem was uncovered while attempting to perform unaligned writes,
> >> which caused flatview_write_continue() to reach the call to
> >> flatview_translate(), which then translated the erroneous address and
> >> caused the subsequent write to fail.
> >>
> >> Fixes: 48564041a7 ("exec: reintroduce MemoryRegion caching")
> >> Co-Developed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >> Signed-off-by: Alberto Faria <afaria@redhat.com>
> >> ---
> >>  softmmu/physmem.c | 18 ++++++++++++++----
> >>  1 file changed, 14 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> >> index 50231bab30..bdde4eb927 100644
> >> --- a/softmmu/physmem.c
> >> +++ b/softmmu/physmem.c
> >> @@ -3420,6 +3420,14 @@ static inline MemoryRegion *address_space_translate_cached(
> >>      return section.mr;
> >>  }
> >>
> >> +/* Converts `addr` from the address space of `cache` to that of `cache->fv`. */
> >> +static inline hwaddr addr_in_cache_to_in_flat_view(MemoryRegionCache *cache,
> >> +                                                   hwaddr addr)
> >> +{
> >> +    hwaddr addr_in_mrs = addr + cache->xlat - cache->mrs.offset_within_region;
> >> +    return addr_in_mrs + cache->mrs.offset_within_address_space;
> >> +}
> >> +
> >>  /* Called from RCU critical section. address_space_read_cached uses this
> >>   * out of line function when the target is an MMIO or IOMMU region.
> >>   */
> >> @@ -3434,8 +3442,9 @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr,
> >>      mr = address_space_translate_cached(cache, addr, &addr1, &l, false,
> >>                                          MEMTXATTRS_UNSPECIFIED);
> >>      return flatview_read_continue(cache->fv,
> >> -                                  addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> >> -                                  addr1, l, mr);
> >> +                                  addr_in_cache_to_in_flat_view(cache, addr),
> >> +                                  MEMTXATTRS_UNSPECIFIED, buf, len, addr1, l,
> >> +                                  mr);
> >>  }
> >>
> >>  /* Called from RCU critical section. address_space_write_cached uses this
> >> @@ -3452,8 +3461,9 @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr,
> >>      mr = address_space_translate_cached(cache, addr, &addr1, &l, true,
> >>                                          MEMTXATTRS_UNSPECIFIED);
> >>      return flatview_write_continue(cache->fv,
> >> -                                   addr, MEMTXATTRS_UNSPECIFIED, buf, len,
> >> -                                   addr1, l, mr);
> >> +                                   addr_in_cache_to_in_flat_view(cache, addr),
> >> +                                   MEMTXATTRS_UNSPECIFIED, buf, len, addr1, l,
> >> +                                   mr);
> >>  }
> >>
> >>  #define ARG1_DECL                MemoryRegionCache *cache
> >> --
> >> 2.37.2
>
> This doesn't seem to be in master yet. Maybe it fell through the cracks?

Ping.

Stefan


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-01-11 14:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-05  0:00 [PATCH v2] softmmu/physmem: Fix address of FlatView access in address_space_(read|write)_cached_slow() Alberto Faria
2022-09-05  7:45 ` Paolo Bonzini
2022-12-02 12:29   ` Alberto Faria
2023-01-11 14:28     ` Stefan Hajnoczi

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).