[v2] softmmu/physmem: Fix input parameters for flatview_access_allowed()

[PATCH v2] softmmu/physmem: Fix input parameters for flatview_access_allowed()

Posted by Zhenzhong Duan 1 year, 9 months ago

The comment of flatview_access_allowed() suggests to pass address
within that memory region, this isn't true in some call sites.

This makes qemu log in flatview_access_allowed() confusing and
potential risk if the input parameter will be checked in the future.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
v2: Fix typo and removed Fixed-by per David

 softmmu/physmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index fb16be57a6c6..214cb04c8fc3 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2850,7 +2850,7 @@ static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
 
     l = len;
     mr = flatview_translate(fv, addr, &addr1, &l, true, attrs);
-    if (!flatview_access_allowed(mr, attrs, addr, len)) {
+    if (!flatview_access_allowed(mr, attrs, addr1, l)) {
         return MEMTX_ACCESS_ERROR;
     }
     return flatview_write_continue(fv, addr, attrs, buf, len,
@@ -2917,7 +2917,7 @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
 
     l = len;
     mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
-    if (!flatview_access_allowed(mr, attrs, addr, len)) {
+    if (!flatview_access_allowed(mr, attrs, addr1, l)) {
         return MEMTX_ACCESS_ERROR;
     }
     return flatview_read_continue(fv, addr, attrs, buf, len,
-- 
2.25.1

RE: [PATCH v2] softmmu/physmem: Fix input parameters for flatview_access_allowed()

Posted by Duan, Zhenzhong 1 year, 6 months ago


>-----Original Message-----
>From: Duan, Zhenzhong <zhenzhong.duan@intel.com>
>Sent: Friday, July 22, 2022 4:46 PM
>To: qemu-devel@nongnu.org
>Cc: pbonzini@redhat.com; peterx@redhat.com; david@redhat.com;
>f4bug@amsat.org
>Subject: [PATCH v2] softmmu/physmem: Fix input parameters for
>flatview_access_allowed()
>
>The comment of flatview_access_allowed() suggests to pass address within
>that memory region, this isn't true in some call sites.
>
>This makes qemu log in flatview_access_allowed() confusing and potential risk
>if the input parameter will be checked in the future.
>
>Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>Reviewed-by: Peter Xu <peterx@redhat.com>
>Reviewed-by: David Hildenbrand <david@redhat.com>
>---
>v2: Fix typo and removed Fixed-by per David
Hi Maintainers,

Can this patch be considered merged as it got reviewed-by and no objection for a long time. Thanks.

Zhenzhong

>
> softmmu/physmem.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/softmmu/physmem.c b/softmmu/physmem.c index
>fb16be57a6c6..214cb04c8fc3 100644
>--- a/softmmu/physmem.c
>+++ b/softmmu/physmem.c
>@@ -2850,7 +2850,7 @@ static MemTxResult flatview_write(FlatView *fv,
>hwaddr addr, MemTxAttrs attrs,
>
>     l = len;
>     mr = flatview_translate(fv, addr, &addr1, &l, true, attrs);
>-    if (!flatview_access_allowed(mr, attrs, addr, len)) {
>+    if (!flatview_access_allowed(mr, attrs, addr1, l)) {
>         return MEMTX_ACCESS_ERROR;
>     }
>     return flatview_write_continue(fv, addr, attrs, buf, len, @@ -2917,7
>+2917,7 @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr,
>
>     l = len;
>     mr = flatview_translate(fv, addr, &addr1, &l, false, attrs);
>-    if (!flatview_access_allowed(mr, attrs, addr, len)) {
>+    if (!flatview_access_allowed(mr, attrs, addr1, l)) {
>         return MEMTX_ACCESS_ERROR;
>     }
>     return flatview_read_continue(fv, addr, attrs, buf, len,
>--
>2.25.1