| Message ID | 3d14987b-278c-be28-be7b-8f3c733fc4e9@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/pci: Fix handling of pci use accessor return codes | expand |
On Sun, 24 Jan 2021 16:35:41 +0100 Heiner Kallweit <hkallweit1@gmail.com> wrote: > The pci user accessors return negative errno's on error. > > Fixes: f572a960a15e ("vfio/pci: Intel IGD host and LCP bridge config space access") > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/vfio/pci/vfio_pci_igd.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c > index 53d97f459..e66dfb017 100644 > --- a/drivers/vfio/pci/vfio_pci_igd.c > +++ b/drivers/vfio/pci/vfio_pci_igd.c > @@ -127,7 +127,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, > > ret = pci_user_read_config_byte(pdev, pos, &val); > if (ret) > - return pcibios_err_to_errno(ret); > + return ret; This is actually not strictly needed, as pcibios_err_to_errno() already keeps errors <= 0 unchanged, so more a cleanup than a fix? Anyway, Reviewed-by: Cornelia Huck <cohuck@redhat.com> > > if (copy_to_user(buf + count - size, &val, 1)) > return -EFAULT;
On 25.01.2021 19:11, Cornelia Huck wrote: > On Sun, 24 Jan 2021 16:35:41 +0100 > Heiner Kallweit <hkallweit1@gmail.com> wrote: > >> The pci user accessors return negative errno's on error. >> >> Fixes: f572a960a15e ("vfio/pci: Intel IGD host and LCP bridge config space access") >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/vfio/pci/vfio_pci_igd.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c >> index 53d97f459..e66dfb017 100644 >> --- a/drivers/vfio/pci/vfio_pci_igd.c >> +++ b/drivers/vfio/pci/vfio_pci_igd.c >> @@ -127,7 +127,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, >> >> ret = pci_user_read_config_byte(pdev, pos, &val); >> if (ret) >> - return pcibios_err_to_errno(ret); >> + return ret; > > This is actually not strictly needed, as pcibios_err_to_errno() already > keeps errors <= 0 unchanged, so more a cleanup than a fix? > I agree. Although I'd argue that the author of the original commit missed the fact the the user accessors return errno's, and the code just works by chance, because of the "good will" of pcibios_err_to_errno(). So up to you whether it's worth it to apply this change also to stable. > Anyway, > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > >> >> if (copy_to_user(buf + count - size, &val, 1)) >> return -EFAULT; >
On Sun, 24 Jan 2021 16:35:41 +0100 Heiner Kallweit <hkallweit1@gmail.com> wrote: > The pci user accessors return negative errno's on error. > > Fixes: f572a960a15e ("vfio/pci: Intel IGD host and LCP bridge config space access") > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/vfio/pci/vfio_pci_igd.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Applied to vfio next branch for v5.12 w/ Connie's R-b. I did drop the fixes tag since pcibios_err_to_errno() has always handled -errno correctly to avoid the unnecessary churn. Thanks, Alex > > diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c > index 53d97f459..e66dfb017 100644 > --- a/drivers/vfio/pci/vfio_pci_igd.c > +++ b/drivers/vfio/pci/vfio_pci_igd.c > @@ -127,7 +127,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, > > ret = pci_user_read_config_byte(pdev, pos, &val); > if (ret) > - return pcibios_err_to_errno(ret); > + return ret; > > if (copy_to_user(buf + count - size, &val, 1)) > return -EFAULT; > @@ -141,7 +141,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, > > ret = pci_user_read_config_word(pdev, pos, &val); > if (ret) > - return pcibios_err_to_errno(ret); > + return ret; > > val = cpu_to_le16(val); > if (copy_to_user(buf + count - size, &val, 2)) > @@ -156,7 +156,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, > > ret = pci_user_read_config_dword(pdev, pos, &val); > if (ret) > - return pcibios_err_to_errno(ret); > + return ret; > > val = cpu_to_le32(val); > if (copy_to_user(buf + count - size, &val, 4)) > @@ -171,7 +171,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, > > ret = pci_user_read_config_word(pdev, pos, &val); > if (ret) > - return pcibios_err_to_errno(ret); > + return ret; > > val = cpu_to_le16(val); > if (copy_to_user(buf + count - size, &val, 2)) > @@ -186,7 +186,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, > > ret = pci_user_read_config_byte(pdev, pos, &val); > if (ret) > - return pcibios_err_to_errno(ret); > + return ret; > > if (copy_to_user(buf + count - size, &val, 1)) > return -EFAULT;
diff --git a/drivers/vfio/pci/vfio_pci_igd.c b/drivers/vfio/pci/vfio_pci_igd.c index 53d97f459..e66dfb017 100644 --- a/drivers/vfio/pci/vfio_pci_igd.c +++ b/drivers/vfio/pci/vfio_pci_igd.c @@ -127,7 +127,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, ret = pci_user_read_config_byte(pdev, pos, &val); if (ret) - return pcibios_err_to_errno(ret); + return ret; if (copy_to_user(buf + count - size, &val, 1)) return -EFAULT; @@ -141,7 +141,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, ret = pci_user_read_config_word(pdev, pos, &val); if (ret) - return pcibios_err_to_errno(ret); + return ret; val = cpu_to_le16(val); if (copy_to_user(buf + count - size, &val, 2)) @@ -156,7 +156,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, ret = pci_user_read_config_dword(pdev, pos, &val); if (ret) - return pcibios_err_to_errno(ret); + return ret; val = cpu_to_le32(val); if (copy_to_user(buf + count - size, &val, 4)) @@ -171,7 +171,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, ret = pci_user_read_config_word(pdev, pos, &val); if (ret) - return pcibios_err_to_errno(ret); + return ret; val = cpu_to_le16(val); if (copy_to_user(buf + count - size, &val, 2)) @@ -186,7 +186,7 @@ static size_t vfio_pci_igd_cfg_rw(struct vfio_pci_device *vdev, ret = pci_user_read_config_byte(pdev, pos, &val); if (ret) - return pcibios_err_to_errno(ret); + return ret; if (copy_to_user(buf + count - size, &val, 1)) return -EFAULT;
The pci user accessors return negative errno's on error. Fixes: f572a960a15e ("vfio/pci: Intel IGD host and LCP bridge config space access") Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/vfio/pci/vfio_pci_igd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)