kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vfio/pci: Fix OpRegion read
@ 2021-11-19  8:14 Zhenyu Wang
  2021-11-21 14:01 ` Colin Xu
  2021-11-24  0:56 ` Colin Xu
  0 siblings, 2 replies; 5+ messages in thread
From: Zhenyu Wang @ 2021-11-19  8:14 UTC (permalink / raw)
  To: kvm; +Cc: Colin Xu, Dmitry Torokhov

This is to fix incorrect pointer arithmetic which caused wrong
OpRegion version returned, then VM driver got error to get wanted
VBT block. We need to be safe to return correct data, so force
pointer type for byte access.

Fixes: 49ba1a2976c8 ("vfio/pci: Add OpRegion 2.0+ Extended VBT support.")
Cc: Colin Xu <colin.xu@gmail.com>
Cc: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/vfio/pci/vfio_pci_igd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c
index 56cd551e0e04..dad6eeed5e80 100644
--- a/drivers/vfio/pci/vfio_pci_igd.c
+++ b/drivers/vfio/pci/vfio_pci_igd.c
@@ -98,7 +98,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
 			version = cpu_to_le16(0x0201);
 
 		if (igd_opregion_shift_copy(buf, &off,
-					    &version + (pos - OPREGION_VERSION),
+					    (u8 *)&version + (pos - OPREGION_VERSION),
 					    &pos, &remaining, bytes))
 			return -EFAULT;
 	}
@@ -121,7 +121,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
 					  OPREGION_SIZE : 0);
 
 		if (igd_opregion_shift_copy(buf, &off,
-					    &rvda + (pos - OPREGION_RVDA),
+					    (u8 *)&rvda + (pos - OPREGION_RVDA),
 					    &pos, &remaining, bytes))
 			return -EFAULT;
 	}
-- 
2.33.1


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

* Re: [PATCH] vfio/pci: Fix OpRegion read
  2021-11-19  8:14 [PATCH] vfio/pci: Fix OpRegion read Zhenyu Wang
@ 2021-11-21 14:01 ` Colin Xu
  2021-11-24  0:56 ` Colin Xu
  1 sibling, 0 replies; 5+ messages in thread
From: Colin Xu @ 2021-11-21 14:01 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: kvm, Dmitry Torokhov

Thanks for the fix.
The implicit ptr cast will incorrectly advance the pointer. Cast to
byte is the correct step.

Best Regards,
Colin

On Fri, Nov 19, 2021 at 4:14 PM Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
>
> This is to fix incorrect pointer arithmetic which caused wrong
> OpRegion version returned, then VM driver got error to get wanted
> VBT block. We need to be safe to return correct data, so force
> pointer type for byte access.
>
> Fixes: 49ba1a2976c8 ("vfio/pci: Add OpRegion 2.0+ Extended VBT support.")
> Cc: Colin Xu <colin.xu@gmail.com>
> Cc: Dmitry Torokhov <dtor@chromium.org>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/vfio/pci/vfio_pci_igd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c
> index 56cd551e0e04..dad6eeed5e80 100644
> --- a/drivers/vfio/pci/vfio_pci_igd.c
> +++ b/drivers/vfio/pci/vfio_pci_igd.c
> @@ -98,7 +98,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
>                         version = cpu_to_le16(0x0201);
>
>                 if (igd_opregion_shift_copy(buf, &off,
> -                                           &version + (pos - OPREGION_VERSION),
> +                                           (u8 *)&version + (pos - OPREGION_VERSION),
>                                             &pos, &remaining, bytes))
>                         return -EFAULT;
>         }
> @@ -121,7 +121,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
>                                           OPREGION_SIZE : 0);
>
>                 if (igd_opregion_shift_copy(buf, &off,
> -                                           &rvda + (pos - OPREGION_RVDA),
> +                                           (u8 *)&rvda + (pos - OPREGION_RVDA),
>                                             &pos, &remaining, bytes))
>                         return -EFAULT;
>         }
> --
> 2.33.1
>

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

* Re: [PATCH] vfio/pci: Fix OpRegion read
  2021-11-19  8:14 [PATCH] vfio/pci: Fix OpRegion read Zhenyu Wang
  2021-11-21 14:01 ` Colin Xu
@ 2021-11-24  0:56 ` Colin Xu
  2021-11-25  5:13   ` Zhenyu Wang
  1 sibling, 1 reply; 5+ messages in thread
From: Colin Xu @ 2021-11-24  0:56 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: kvm, Dmitry Torokhov

Thanks and appreciated for the fix.

Acked-by: Colin Xu <colin.xu@gmail.com>

On Fri, Nov 19, 2021 at 4:14 PM Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
>
> This is to fix incorrect pointer arithmetic which caused wrong
> OpRegion version returned, then VM driver got error to get wanted
> VBT block. We need to be safe to return correct data, so force
> pointer type for byte access.
>
> Fixes: 49ba1a2976c8 ("vfio/pci: Add OpRegion 2.0+ Extended VBT support.")
> Cc: Colin Xu <colin.xu@gmail.com>
> Cc: Dmitry Torokhov <dtor@chromium.org>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/vfio/pci/vfio_pci_igd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c
> index 56cd551e0e04..dad6eeed5e80 100644
> --- a/drivers/vfio/pci/vfio_pci_igd.c
> +++ b/drivers/vfio/pci/vfio_pci_igd.c
> @@ -98,7 +98,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
>                         version = cpu_to_le16(0x0201);
>
>                 if (igd_opregion_shift_copy(buf, &off,
> -                                           &version + (pos - OPREGION_VERSION),
> +                                           (u8 *)&version + (pos - OPREGION_VERSION),
>                                             &pos, &remaining, bytes))
>                         return -EFAULT;
>         }
> @@ -121,7 +121,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
>                                           OPREGION_SIZE : 0);
>
>                 if (igd_opregion_shift_copy(buf, &off,
> -                                           &rvda + (pos - OPREGION_RVDA),
> +                                           (u8 *)&rvda + (pos - OPREGION_RVDA),
>                                             &pos, &remaining, bytes))
>                         return -EFAULT;
>         }
> --
> 2.33.1
>

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

* [PATCH] vfio/pci: Fix OpRegion read
  2021-11-24  0:56 ` Colin Xu
@ 2021-11-25  5:13   ` Zhenyu Wang
  2021-11-30 19:30     ` Alex Williamson
  0 siblings, 1 reply; 5+ messages in thread
From: Zhenyu Wang @ 2021-11-25  5:13 UTC (permalink / raw)
  To: kvm; +Cc: Colin Xu, Alex Williamson, Dmitry Torokhov, Xu, Terrence, Gao, Fred

This is to fix incorrect pointer arithmetic which caused wrong
OpRegion version returned, then VM driver got error to get wanted
VBT block. We need to be safe to return correct data, so force
pointer type for byte access.

Fixes: 49ba1a2976c8 ("vfio/pci: Add OpRegion 2.0+ Extended VBT support.")
Cc: Colin Xu <colin.xu@gmail.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dmitry Torokhov <dtor@chromium.org>
Cc: "Xu, Terrence" <terrence.xu@intel.com>
Cc: "Gao, Fred" <fred.gao@intel.com>
Acked-by: Colin Xu <colin.xu@gmail.com>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/vfio/pci/vfio_pci_igd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c
index 56cd551e0e04..dad6eeed5e80 100644
--- a/drivers/vfio/pci/vfio_pci_igd.c
+++ b/drivers/vfio/pci/vfio_pci_igd.c
@@ -98,7 +98,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
 			version = cpu_to_le16(0x0201);
 
 		if (igd_opregion_shift_copy(buf, &off,
-					    &version + (pos - OPREGION_VERSION),
+					    (u8 *)&version + (pos - OPREGION_VERSION),
 					    &pos, &remaining, bytes))
 			return -EFAULT;
 	}
@@ -121,7 +121,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
 					  OPREGION_SIZE : 0);
 
 		if (igd_opregion_shift_copy(buf, &off,
-					    &rvda + (pos - OPREGION_RVDA),
+					    (u8 *)&rvda + (pos - OPREGION_RVDA),
 					    &pos, &remaining, bytes))
 			return -EFAULT;
 	}
-- 
2.33.1


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

* Re: [PATCH] vfio/pci: Fix OpRegion read
  2021-11-25  5:13   ` Zhenyu Wang
@ 2021-11-30 19:30     ` Alex Williamson
  0 siblings, 0 replies; 5+ messages in thread
From: Alex Williamson @ 2021-11-30 19:30 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: kvm, Colin Xu, Dmitry Torokhov, Xu, Terrence, Gao, Fred

On Thu, 25 Nov 2021 13:13:28 +0800
Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> This is to fix incorrect pointer arithmetic which caused wrong
> OpRegion version returned, then VM driver got error to get wanted
> VBT block. We need to be safe to return correct data, so force
> pointer type for byte access.
> 
> Fixes: 49ba1a2976c8 ("vfio/pci: Add OpRegion 2.0+ Extended VBT support.")
> Cc: Colin Xu <colin.xu@gmail.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Dmitry Torokhov <dtor@chromium.org>
> Cc: "Xu, Terrence" <terrence.xu@intel.com>
> Cc: "Gao, Fred" <fred.gao@intel.com>
> Acked-by: Colin Xu <colin.xu@gmail.com>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/vfio/pci/vfio_pci_igd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c
> index 56cd551e0e04..dad6eeed5e80 100644
> --- a/drivers/vfio/pci/vfio_pci_igd.c
> +++ b/drivers/vfio/pci/vfio_pci_igd.c
> @@ -98,7 +98,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
>  			version = cpu_to_le16(0x0201);
>  
>  		if (igd_opregion_shift_copy(buf, &off,
> -					    &version + (pos - OPREGION_VERSION),
> +					    (u8 *)&version + (pos - OPREGION_VERSION),
>  					    &pos, &remaining, bytes))
>  			return -EFAULT;
>  	}
> @@ -121,7 +121,7 @@ static ssize_t vfio_pci_igd_rw(struct vfio_pci_core_device *vdev,
>  					  OPREGION_SIZE : 0);
>  
>  		if (igd_opregion_shift_copy(buf, &off,
> -					    &rvda + (pos - OPREGION_RVDA),
> +					    (u8 *)&rvda + (pos - OPREGION_RVDA),
>  					    &pos, &remaining, bytes))
>  			return -EFAULT;
>  	}

Applied to vfio for-linus branch for v5.16.  Thanks,

Alex


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

end of thread, other threads:[~2021-11-30 19:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19  8:14 [PATCH] vfio/pci: Fix OpRegion read Zhenyu Wang
2021-11-21 14:01 ` Colin Xu
2021-11-24  0:56 ` Colin Xu
2021-11-25  5:13   ` Zhenyu Wang
2021-11-30 19:30     ` Alex Williamson

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