* Request VFIO inclusion in linux-next
@ 2012-06-26 4:55 ` Alex Williamson
0 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-26 4:55 UTC (permalink / raw)
To: Stephen Rothwell, Linus Torvalds, Andrew Morton, Greg Kroah-Hartman
Cc: kvm, Alexey Kardashevskiy, linux-pci, linux-kernel, qemu-devel,
chrisw, iommu, linux-next-u79uwXL29TY76Z2rM5mHXA,
Benjamin Herrenschmidt, David Gibson
Hi,
VFIO has been kicking around for well over a year now and has been
posted numerous times for review. The pre-requirements are finally
available in linux-next (or will be in the 20120626 build) so I'd like
to request a new branch be included in linux-next with a goal of being
accepted into v3.6.
VFIO is a userspace driver interface designed to support assignment of
devices into virtual machines using IOMMU level access control. This
IOMMU requirement, secure resource access, and flexible interrupt
support make VFIO unique from existing drivers, like UIO. VFIO supports
modular backends for both IOMMU and device access. Initial backends are
included for PCI device assignment using the IOMMU API in a manner
compatible with x86 device assignment. POWER support is also under
development, making use of the same PCI device backend, but adding new
IOMMU support for their platforms.
As with previous versions of VFIO, Qemu is targeted as a primary user
and a working development tree including vfio-pci support can be found
here:
git://github.com/awilliam/qemu-vfio.git iommu-group-vfio
Eventually we hope VFIO can deprecate the x86, PCI-specific device
assignment currently used by KVM.
The info for linux-next:
Tree: git://github.com/awilliam/linux-vfio.git
Branch: next
Contact: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
This branch should be applied after both Bjorn's PCI next branch and
Joerg's IOMMU next branch and contains the following changes:
Documentation/ioctl/ioctl-number.txt | 1
Documentation/vfio.txt | 315 +++++++
MAINTAINERS | 8
drivers/Kconfig | 2
drivers/Makefile | 1
drivers/vfio/Kconfig | 16
drivers/vfio/Makefile | 3
drivers/vfio/pci/Kconfig | 8
drivers/vfio/pci/Makefile | 4
drivers/vfio/pci/vfio_pci.c | 565 ++++++++++++
drivers/vfio/pci/vfio_pci_config.c | 1528 +++++++++++++++++++++++++++++++++++
drivers/vfio/pci/vfio_pci_intrs.c | 727 ++++++++++++++++
drivers/vfio/pci/vfio_pci_private.h | 91 ++
drivers/vfio/pci/vfio_pci_rdwr.c | 269 ++++++
drivers/vfio/vfio.c | 1420 ++++++++++++++++++++++++++++++++
drivers/vfio/vfio_iommu_type1.c | 754 +++++++++++++++++
include/linux/vfio.h | 445 ++++++++++
17 files changed, 6157 insertions(+)
If there are any objections to including this, please speak now. If
anything looks amiss in the branch, let me know. I've never hosted a
next branch. Review comments welcome and I'll be glad to post the
series in email again if requested. Thanks,
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
@ 2012-06-26 21:17 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-26 21:17 UTC (permalink / raw)
To: Alex Williamson
Cc: Stephen Rothwell, Linus Torvalds, Andrew Morton,
Greg Kroah-Hartman, linux-kernel, kvm, linux-pci, iommu,
qemu-devel, Alexey Kardashevskiy, David Gibson, chrisw, Roedel,
Joerg, linux-next
On Mon, 2012-06-25 at 22:55 -0600, Alex Williamson wrote:
> Hi,
>
> VFIO has been kicking around for well over a year now and has been
> posted numerous times for review. The pre-requirements are finally
> available in linux-next (or will be in the 20120626 build) so I'd like
> to request a new branch be included in linux-next with a goal of being
> accepted into v3.6.
Ack. Let's get that in, it's been simmering for too long and we'll need
that to do PCI pass-through on KVM powerpc.
Cheers,
Ben.
> VFIO is a userspace driver interface designed to support assignment of
> devices into virtual machines using IOMMU level access control. This
> IOMMU requirement, secure resource access, and flexible interrupt
> support make VFIO unique from existing drivers, like UIO. VFIO supports
> modular backends for both IOMMU and device access. Initial backends are
> included for PCI device assignment using the IOMMU API in a manner
> compatible with x86 device assignment. POWER support is also under
> development, making use of the same PCI device backend, but adding new
> IOMMU support for their platforms.
>
> As with previous versions of VFIO, Qemu is targeted as a primary user
> and a working development tree including vfio-pci support can be found
> here:
>
> git://github.com/awilliam/qemu-vfio.git iommu-group-vfio
>
> Eventually we hope VFIO can deprecate the x86, PCI-specific device
> assignment currently used by KVM.
>
> The info for linux-next:
>
> Tree: git://github.com/awilliam/linux-vfio.git
> Branch: next
> Contact: Alex Williamson <alex.williamson@redhat.com>
>
> This branch should be applied after both Bjorn's PCI next branch and
> Joerg's IOMMU next branch and contains the following changes:
>
> Documentation/ioctl/ioctl-number.txt | 1
> Documentation/vfio.txt | 315 +++++++
> MAINTAINERS | 8
> drivers/Kconfig | 2
> drivers/Makefile | 1
> drivers/vfio/Kconfig | 16
> drivers/vfio/Makefile | 3
> drivers/vfio/pci/Kconfig | 8
> drivers/vfio/pci/Makefile | 4
> drivers/vfio/pci/vfio_pci.c | 565 ++++++++++++
> drivers/vfio/pci/vfio_pci_config.c | 1528 +++++++++++++++++++++++++++++++++++
> drivers/vfio/pci/vfio_pci_intrs.c | 727 ++++++++++++++++
> drivers/vfio/pci/vfio_pci_private.h | 91 ++
> drivers/vfio/pci/vfio_pci_rdwr.c | 269 ++++++
> drivers/vfio/vfio.c | 1420 ++++++++++++++++++++++++++++++++
> drivers/vfio/vfio_iommu_type1.c | 754 +++++++++++++++++
> include/linux/vfio.h | 445 ++++++++++
> 17 files changed, 6157 insertions(+)
>
> If there are any objections to including this, please speak now. If
> anything looks amiss in the branch, let me know. I've never hosted a
> next branch. Review comments welcome and I'll be glad to post the
> series in email again if requested. Thanks,
>
> Alex
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
@ 2012-06-26 21:17 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 32+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-26 21:17 UTC (permalink / raw)
To: Alex Williamson
Cc: Stephen Rothwell, kvm, Alexey Kardashevskiy, Greg Kroah-Hartman,
linux-kernel, qemu-devel, chrisw, iommu,
linux-next-u79uwXL29TY76Z2rM5mHXA, linux-pci, Andrew Morton,
Linus Torvalds, David Gibson
On Mon, 2012-06-25 at 22:55 -0600, Alex Williamson wrote:
> Hi,
>
> VFIO has been kicking around for well over a year now and has been
> posted numerous times for review. The pre-requirements are finally
> available in linux-next (or will be in the 20120626 build) so I'd like
> to request a new branch be included in linux-next with a goal of being
> accepted into v3.6.
Ack. Let's get that in, it's been simmering for too long and we'll need
that to do PCI pass-through on KVM powerpc.
Cheers,
Ben.
> VFIO is a userspace driver interface designed to support assignment of
> devices into virtual machines using IOMMU level access control. This
> IOMMU requirement, secure resource access, and flexible interrupt
> support make VFIO unique from existing drivers, like UIO. VFIO supports
> modular backends for both IOMMU and device access. Initial backends are
> included for PCI device assignment using the IOMMU API in a manner
> compatible with x86 device assignment. POWER support is also under
> development, making use of the same PCI device backend, but adding new
> IOMMU support for their platforms.
>
> As with previous versions of VFIO, Qemu is targeted as a primary user
> and a working development tree including vfio-pci support can be found
> here:
>
> git://github.com/awilliam/qemu-vfio.git iommu-group-vfio
>
> Eventually we hope VFIO can deprecate the x86, PCI-specific device
> assignment currently used by KVM.
>
> The info for linux-next:
>
> Tree: git://github.com/awilliam/linux-vfio.git
> Branch: next
> Contact: Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> This branch should be applied after both Bjorn's PCI next branch and
> Joerg's IOMMU next branch and contains the following changes:
>
> Documentation/ioctl/ioctl-number.txt | 1
> Documentation/vfio.txt | 315 +++++++
> MAINTAINERS | 8
> drivers/Kconfig | 2
> drivers/Makefile | 1
> drivers/vfio/Kconfig | 16
> drivers/vfio/Makefile | 3
> drivers/vfio/pci/Kconfig | 8
> drivers/vfio/pci/Makefile | 4
> drivers/vfio/pci/vfio_pci.c | 565 ++++++++++++
> drivers/vfio/pci/vfio_pci_config.c | 1528 +++++++++++++++++++++++++++++++++++
> drivers/vfio/pci/vfio_pci_intrs.c | 727 ++++++++++++++++
> drivers/vfio/pci/vfio_pci_private.h | 91 ++
> drivers/vfio/pci/vfio_pci_rdwr.c | 269 ++++++
> drivers/vfio/vfio.c | 1420 ++++++++++++++++++++++++++++++++
> drivers/vfio/vfio_iommu_type1.c | 754 +++++++++++++++++
> include/linux/vfio.h | 445 ++++++++++
> 17 files changed, 6157 insertions(+)
>
> If there are any objections to including this, please speak now. If
> anything looks amiss in the branch, let me know. I've never hosted a
> next branch. Review comments welcome and I'll be glad to post the
> series in email again if requested. Thanks,
>
> Alex
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
2012-06-26 4:55 ` Alex Williamson
(?)
(?)
@ 2012-06-26 23:50 ` Stephen Rothwell
-1 siblings, 0 replies; 32+ messages in thread
From: Stephen Rothwell @ 2012-06-26 23:50 UTC (permalink / raw)
To: Alex Williamson
Cc: Linus Torvalds, Andrew Morton, Greg Kroah-Hartman, linux-kernel,
kvm, linux-pci, iommu, qemu-devel, Benjamin Herrenschmidt,
Alexey Kardashevskiy, David Gibson, chrisw, Roedel, Joerg,
linux-next, Bjorn Helgaas
[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]
Hi Alex,
On Mon, 25 Jun 2012 22:55:52 -0600 Alex Williamson <alex.williamson@redhat.com> wrote:
>
> VFIO has been kicking around for well over a year now and has been
> posted numerous times for review. The pre-requirements are finally
> available in linux-next (or will be in the 20120626 build) so I'd like
> to request a new branch be included in linux-next with a goal of being
> accepted into v3.6.
>
> The info for linux-next:
>
> Tree: git://github.com/awilliam/linux-vfio.git
> Branch: next
> Contact: Alex Williamson <alex.williamson@redhat.com>
>
> This branch should be applied after both Bjorn's PCI next branch and
> Joerg's IOMMU next branch
I have added this from today. Since this tree merges (parts of) the pci
and iommu trees, I am hoping that those are to remain stable - if not,
then you need to stay alert if they are rebased.
Thanks for adding your subsystem tree as a participant of linux-next. As
you may know, this is not a judgment of your code. The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window.
You will need to ensure that the patches/commits in your tree/series have
been:
* submitted under GPL v2 (or later) and include the Contributor's
Signed-off-by,
* posted to the relevant mailing list,
* reviewed by you (or another maintainer of your subsystem tree),
* successfully unit tested, and
* destined for the current or next Linux merge window.
Basically, this should be just what you would send to Linus (or ask him
to fetch). It is allowed to be rebased if you deem it necessary.
--
Cheers,
Stephen Rothwell
sfr@canb.auug.org.au
Legal Stuff:
By participating in linux-next, your subsystem tree contributions are
public and will be included in the linux-next trees. You may be sent
e-mail messages indicating errors or other issues when the
patches/commits from your subsystem tree are merged and tested in
linux-next. These messages may also be cross-posted to the linux-next
mailing list, the linux-kernel mailing list, etc. The linux-next tree
project and IBM (my employer) make no warranties regarding the linux-next
project, the testing procedures, the results, the e-mails, etc. If you
don't agree to these ground rules, let me know and I'll remove your tree
from participation in linux-next.
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: Request VFIO inclusion in linux-next
@ 2012-06-27 12:37 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-27 12:37 UTC (permalink / raw)
To: Alex Williamson
Cc: Stephen Rothwell, Linus Torvalds, Andrew Morton,
Greg Kroah-Hartman, linux-kernel, kvm, linux-pci, iommu,
qemu-devel, Benjamin Herrenschmidt, Alexey Kardashevskiy,
David Gibson, chrisw, Roedel, Joerg, linux-next
On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> Hi,
>
> VFIO has been kicking around for well over a year now and has been
> posted numerous times for review. The pre-requirements are finally
> available in linux-next (or will be in the 20120626 build) so I'd like
> to request a new branch be included in linux-next with a goal of being
> accepted into v3.6.
>
Could you run Sparse over the driver?
http://lwn.net/Articles/205624/
It reports a bunch of endian problems. Some are definitely bugs
like:
*prev |= cpu_to_le32((u32)epos << 20);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
@ 2012-06-27 12:37 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-27 12:37 UTC (permalink / raw)
To: Alex Williamson
Cc: Stephen Rothwell, Benjamin Herrenschmidt, kvm,
Alexey Kardashevskiy, Greg Kroah-Hartman, linux-kernel,
qemu-devel, chrisw, iommu, linux-next-u79uwXL29TY76Z2rM5mHXA,
linux-pci, Andrew Morton, Linus Torvalds, David Gibson
On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> Hi,
>
> VFIO has been kicking around for well over a year now and has been
> posted numerous times for review. The pre-requirements are finally
> available in linux-next (or will be in the 20120626 build) so I'd like
> to request a new branch be included in linux-next with a goal of being
> accepted into v3.6.
>
Could you run Sparse over the driver?
http://lwn.net/Articles/205624/
It reports a bunch of endian problems. Some are definitely bugs
like:
*prev |= cpu_to_le32((u32)epos << 20);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
2012-06-27 12:37 ` Dan Carpenter
@ 2012-06-27 19:23 ` Alex Williamson
-1 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-27 19:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Stephen Rothwell, Linus Torvalds, Andrew Morton,
Greg Kroah-Hartman, linux-kernel, kvm, linux-pci, iommu,
qemu-devel, Benjamin Herrenschmidt, Alexey Kardashevskiy,
David Gibson, chrisw, Roedel, Joerg, linux-next
On Wed, 2012-06-27 at 15:37 +0300, Dan Carpenter wrote:
> On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> > Hi,
> >
> > VFIO has been kicking around for well over a year now and has been
> > posted numerous times for review. The pre-requirements are finally
> > available in linux-next (or will be in the 20120626 build) so I'd like
> > to request a new branch be included in linux-next with a goal of being
> > accepted into v3.6.
> >
>
> Could you run Sparse over the driver?
> http://lwn.net/Articles/205624/
>
> It reports a bunch of endian problems. Some are definitely bugs
> like:
> *prev |= cpu_to_le32((u32)epos << 20);
Done. Mostly just adding annotation, but it did find a bug or two.
Should be cleaned up in tomorrow's next. Thanks for the report,
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
@ 2012-06-27 19:23 ` Alex Williamson
0 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-27 19:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Stephen Rothwell, Benjamin Herrenschmidt, kvm,
Alexey Kardashevskiy, Greg Kroah-Hartman, linux-kernel,
qemu-devel, chrisw, iommu, linux-next-u79uwXL29TY76Z2rM5mHXA,
linux-pci, Andrew Morton, Linus Torvalds, David Gibson
On Wed, 2012-06-27 at 15:37 +0300, Dan Carpenter wrote:
> On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> > Hi,
> >
> > VFIO has been kicking around for well over a year now and has been
> > posted numerous times for review. The pre-requirements are finally
> > available in linux-next (or will be in the 20120626 build) so I'd like
> > to request a new branch be included in linux-next with a goal of being
> > accepted into v3.6.
> >
>
> Could you run Sparse over the driver?
> http://lwn.net/Articles/205624/
>
> It reports a bunch of endian problems. Some are definitely bugs
> like:
> *prev |= cpu_to_le32((u32)epos << 20);
Done. Mostly just adding annotation, but it did find a bug or two.
Should be cleaned up in tomorrow's next. Thanks for the report,
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
@ 2012-06-28 6:44 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 6:44 UTC (permalink / raw)
To: Alex Williamson
Cc: Stephen Rothwell, Linus Torvalds, Andrew Morton,
Greg Kroah-Hartman, linux-kernel, kvm, linux-pci, iommu,
qemu-devel, Benjamin Herrenschmidt, Alexey Kardashevskiy,
David Gibson, chrisw, Roedel, Joerg, linux-next
On Wed, Jun 27, 2012 at 01:23:23PM -0600, Alex Williamson wrote:
> On Wed, 2012-06-27 at 15:37 +0300, Dan Carpenter wrote:
> > On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> > > Hi,
> > >
> > > VFIO has been kicking around for well over a year now and has been
> > > posted numerous times for review. The pre-requirements are finally
> > > available in linux-next (or will be in the 20120626 build) so I'd like
> > > to request a new branch be included in linux-next with a goal of being
> > > accepted into v3.6.
> > >
> >
> > Could you run Sparse over the driver?
> > http://lwn.net/Articles/205624/
> >
> > It reports a bunch of endian problems. Some are definitely bugs
> > like:
> > *prev |= cpu_to_le32((u32)epos << 20);
>
> Done. Mostly just adding annotation, but it did find a bug or two.
> Should be cleaned up in tomorrow's next. Thanks for the report,
>
Cool. I have some Smatch fixes as well. I've updated them to apply
on top of today's linux-next.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
@ 2012-06-28 6:44 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 6:44 UTC (permalink / raw)
To: Alex Williamson
Cc: Stephen Rothwell, Benjamin Herrenschmidt, kvm,
Alexey Kardashevskiy, Greg Kroah-Hartman, linux-kernel,
qemu-devel, chrisw, iommu, linux-next-u79uwXL29TY76Z2rM5mHXA,
linux-pci, Andrew Morton, Linus Torvalds, David Gibson
On Wed, Jun 27, 2012 at 01:23:23PM -0600, Alex Williamson wrote:
> On Wed, 2012-06-27 at 15:37 +0300, Dan Carpenter wrote:
> > On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> > > Hi,
> > >
> > > VFIO has been kicking around for well over a year now and has been
> > > posted numerous times for review. The pre-requirements are finally
> > > available in linux-next (or will be in the 20120626 build) so I'd like
> > > to request a new branch be included in linux-next with a goal of being
> > > accepted into v3.6.
> > >
> >
> > Could you run Sparse over the driver?
> > http://lwn.net/Articles/205624/
> >
> > It reports a bunch of endian problems. Some are definitely bugs
> > like:
> > *prev |= cpu_to_le32((u32)epos << 20);
>
> Done. Mostly just adding annotation, but it did find a bug or two.
> Should be cleaned up in tomorrow's next. Thanks for the report,
>
Cool. I have some Smatch fixes as well. I've updated them to apply
on top of today's linux-next.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
2012-06-27 12:37 ` Dan Carpenter
@ 2012-07-02 3:41 ` Alexey Kardashevskiy
-1 siblings, 0 replies; 32+ messages in thread
From: Alexey Kardashevskiy @ 2012-07-02 3:41 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alex Williamson, Stephen Rothwell, Linus Torvalds, Andrew Morton,
Greg Kroah-Hartman, linux-kernel, kvm, linux-pci, iommu,
qemu-devel, Benjamin Herrenschmidt, David Gibson, chrisw, Roedel,
Joerg, linux-next
On 27/06/12 22:37, Dan Carpenter wrote:
> On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
>> Hi,
>>
>> VFIO has been kicking around for well over a year now and has been
>> posted numerous times for review. The pre-requirements are finally
>> available in linux-next (or will be in the 20120626 build) so I'd like
>> to request a new branch be included in linux-next with a goal of being
>> accepted into v3.6.
>>
>
> Could you run Sparse over the driver?
> http://lwn.net/Articles/205624/
>
> It reports a bunch of endian problems. Some are definitely bugs
> like:
> *prev |= cpu_to_le32((u32)epos << 20);
What is wrong here?
--
Alexey
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] Request VFIO inclusion in linux-next
@ 2012-07-02 3:41 ` Alexey Kardashevskiy
0 siblings, 0 replies; 32+ messages in thread
From: Alexey Kardashevskiy @ 2012-07-02 3:41 UTC (permalink / raw)
To: Dan Carpenter
Cc: Stephen Rothwell, kvm, Greg Kroah-Hartman, Roedel, Joerg,
linux-kernel, iommu, qemu-devel, chrisw, Alex Williamson,
linux-next, linux-pci, Andrew Morton, Linus Torvalds,
David Gibson
On 27/06/12 22:37, Dan Carpenter wrote:
> On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
>> Hi,
>>
>> VFIO has been kicking around for well over a year now and has been
>> posted numerous times for review. The pre-requirements are finally
>> available in linux-next (or will be in the 20120626 build) so I'd like
>> to request a new branch be included in linux-next with a goal of being
>> accepted into v3.6.
>>
>
> Could you run Sparse over the driver?
> http://lwn.net/Articles/205624/
>
> It reports a bunch of endian problems. Some are definitely bugs
> like:
> *prev |= cpu_to_le32((u32)epos << 20);
What is wrong here?
--
Alexey
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
@ 2012-07-02 4:14 ` Alex Williamson
0 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-07-02 4:14 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Dan Carpenter, Stephen Rothwell, Linus Torvalds, Andrew Morton,
Greg Kroah-Hartman, linux-kernel, kvm, linux-pci, iommu,
qemu-devel, Benjamin Herrenschmidt, David Gibson, chrisw, Roedel,
Joerg, linux-next
On Mon, 2012-07-02 at 13:41 +1000, Alexey Kardashevskiy wrote:
> On 27/06/12 22:37, Dan Carpenter wrote:
> > On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> >> Hi,
> >>
> >> VFIO has been kicking around for well over a year now and has been
> >> posted numerous times for review. The pre-requirements are finally
> >> available in linux-next (or will be in the 20120626 build) so I'd like
> >> to request a new branch be included in linux-next with a goal of being
> >> accepted into v3.6.
> >>
> >
> > Could you run Sparse over the driver?
> > http://lwn.net/Articles/205624/
> >
> > It reports a bunch of endian problems. Some are definitely bugs
> > like:
> > *prev |= cpu_to_le32((u32)epos << 20);
>
>
> What is wrong here?
I believe the only thing wrong here was that prev was a u32* instead of
a __le32*. The new version in my tree has much better endian annotation
after going through all the sparse errors. The only bug I found in the
cleanup was the handling of rbar. It was missing the le32_to_cpu as we
copied it out of vconfig. This is later used with
pci_user_write_config_dword, so it needs to be in native endian.
Thanks,
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] Request VFIO inclusion in linux-next
@ 2012-07-02 4:14 ` Alex Williamson
0 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-07-02 4:14 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Stephen Rothwell, kvm, Greg Kroah-Hartman, Roedel, Joerg,
linux-kernel, qemu-devel, chrisw, iommu, linux-next, linux-pci,
Andrew Morton, Linus Torvalds, Dan Carpenter, David Gibson
On Mon, 2012-07-02 at 13:41 +1000, Alexey Kardashevskiy wrote:
> On 27/06/12 22:37, Dan Carpenter wrote:
> > On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> >> Hi,
> >>
> >> VFIO has been kicking around for well over a year now and has been
> >> posted numerous times for review. The pre-requirements are finally
> >> available in linux-next (or will be in the 20120626 build) so I'd like
> >> to request a new branch be included in linux-next with a goal of being
> >> accepted into v3.6.
> >>
> >
> > Could you run Sparse over the driver?
> > http://lwn.net/Articles/205624/
> >
> > It reports a bunch of endian problems. Some are definitely bugs
> > like:
> > *prev |= cpu_to_le32((u32)epos << 20);
>
>
> What is wrong here?
I believe the only thing wrong here was that prev was a u32* instead of
a __le32*. The new version in my tree has much better endian annotation
after going through all the sparse errors. The only bug I found in the
cleanup was the handling of rbar. It was missing the le32_to_cpu as we
copied it out of vconfig. This is later used with
pci_user_write_config_dword, so it needs to be in native endian.
Thanks,
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: Request VFIO inclusion in linux-next
@ 2012-07-02 4:14 ` Alex Williamson
0 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-07-02 4:14 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Stephen Rothwell, Benjamin Herrenschmidt, kvm,
Greg Kroah-Hartman, linux-kernel, qemu-devel, chrisw, iommu,
linux-next-u79uwXL29TY76Z2rM5mHXA, linux-pci, Andrew Morton,
Linus Torvalds, Dan Carpenter, David Gibson
On Mon, 2012-07-02 at 13:41 +1000, Alexey Kardashevskiy wrote:
> On 27/06/12 22:37, Dan Carpenter wrote:
> > On Mon, Jun 25, 2012 at 10:55:52PM -0600, Alex Williamson wrote:
> >> Hi,
> >>
> >> VFIO has been kicking around for well over a year now and has been
> >> posted numerous times for review. The pre-requirements are finally
> >> available in linux-next (or will be in the 20120626 build) so I'd like
> >> to request a new branch be included in linux-next with a goal of being
> >> accepted into v3.6.
> >>
> >
> > Could you run Sparse over the driver?
> > http://lwn.net/Articles/205624/
> >
> > It reports a bunch of endian problems. Some are definitely bugs
> > like:
> > *prev |= cpu_to_le32((u32)epos << 20);
>
>
> What is wrong here?
I believe the only thing wrong here was that prev was a u32* instead of
a __le32*. The new version in my tree has much better endian annotation
after going through all the sparse errors. The only bug I found in the
cleanup was the handling of rbar. It was missing the le32_to_cpu as we
copied it out of vconfig. This is later used with
pci_user_write_config_dword, so it needs to be in native endian.
Thanks,
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* [patch 1/3] vfio: signedness bug in vfio_config_do_rw()
2012-06-26 4:55 ` Alex Williamson
@ 2012-06-28 6:44 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 6:44 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, linux-kernel, kernel-janitors
The "count" variable is unsigned here so the test for errors doesn't
work.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index a4f7321..10bc6a8 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -1487,7 +1487,7 @@ static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
if (perm->readfn) {
count = perm->readfn(vdev, *ppos, count,
perm, offset, &val);
- if (count < 0)
+ if ((ssize_t)count < 0)
return count;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread* [patch 1/3] vfio: signedness bug in vfio_config_do_rw()
@ 2012-06-28 6:44 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 6:44 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, linux-kernel, kernel-janitors
The "count" variable is unsigned here so the test for errors doesn't
work.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index a4f7321..10bc6a8 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -1487,7 +1487,7 @@ static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
if (perm->readfn) {
count = perm->readfn(vdev, *ppos, count,
perm, offset, &val);
- if (count < 0)
+ if ((ssize_t)count < 0)
return count;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [patch 1/3] vfio: signedness bug in vfio_config_do_rw()
2012-06-28 6:44 ` Dan Carpenter
(?)
@ 2012-06-28 7:15 ` walter harms
2012-06-28 8:07 ` Dan Carpenter
-1 siblings, 1 reply; 32+ messages in thread
From: walter harms @ 2012-06-28 7:15 UTC (permalink / raw)
To: kernel-janitors
Am 28.06.2012 08:44, schrieb Dan Carpenter:
> The "count" variable is unsigned here so the test for errors doesn't
> work.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index a4f7321..10bc6a8 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -1487,7 +1487,7 @@ static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
> if (perm->readfn) {
> count = perm->readfn(vdev, *ppos, count,
> perm, offset, &val);
> - if (count < 0)
> + if ((ssize_t)count < 0)
> return count;
> }
>
hi,
I can only find a few references to vfio_config_do_rw(). From the patchit seems to me this is a wrapper
for perm->readfn since both return ssize_t so why i count unsigned in the first place ?
re,
wh
^ permalink raw reply [flat|nested] 32+ messages in thread* [patch 1/3 v2] vfio: signedness bug in vfio_config_do_rw()
2012-06-28 7:15 ` walter harms
@ 2012-06-28 8:07 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 8:07 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, linux-kernel, kernel-janitors, walter harms
The "count" variable needs to be signed here because we use it to store
negative error codes.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Just declare count as signed.
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index a4f7321..2e00aa8 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -1419,7 +1419,7 @@ void vfio_config_free(struct vfio_pci_device *vdev)
}
static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
- size_t count, loff_t *ppos, bool iswrite)
+ ssize_t count, loff_t *ppos, bool iswrite)
{
struct pci_dev *pdev = vdev->pdev;
struct perm_bits *perm;
^ permalink raw reply related [flat|nested] 32+ messages in thread* [patch 1/3 v2] vfio: signedness bug in vfio_config_do_rw()
@ 2012-06-28 8:07 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 8:07 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, linux-kernel, kernel-janitors, walter harms
The "count" variable needs to be signed here because we use it to store
negative error codes.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Just declare count as signed.
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index a4f7321..2e00aa8 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -1419,7 +1419,7 @@ void vfio_config_free(struct vfio_pci_device *vdev)
}
static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
- size_t count, loff_t *ppos, bool iswrite)
+ ssize_t count, loff_t *ppos, bool iswrite)
{
struct pci_dev *pdev = vdev->pdev;
struct perm_bits *perm;
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [patch 1/3 v2] vfio: signedness bug in vfio_config_do_rw()
2012-06-28 8:07 ` Dan Carpenter
@ 2012-06-28 22:24 ` Alex Williamson
-1 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-28 22:24 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvm, linux-kernel, kernel-janitors, walter harms
On Thu, 2012-06-28 at 11:07 +0300, Dan Carpenter wrote:
> The "count" variable needs to be signed here because we use it to store
> negative error codes.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Just declare count as signed.
>
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index a4f7321..2e00aa8 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -1419,7 +1419,7 @@ void vfio_config_free(struct vfio_pci_device *vdev)
> }
>
> static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
> - size_t count, loff_t *ppos, bool iswrite)
> + ssize_t count, loff_t *ppos, bool iswrite)
> {
> struct pci_dev *pdev = vdev->pdev;
> struct perm_bits *perm;
signed doesn't seem right for count since just below this chunk we do:
if (*ppos < 0 || *ppos + count > pdev->cfg_size)
return -EFAULT;
So then we have to start testing for negative count. I've added a
ssize_t variable for return that should clear things up. Thanks for the
report!
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [patch 1/3 v2] vfio: signedness bug in vfio_config_do_rw()
@ 2012-06-28 22:24 ` Alex Williamson
0 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-28 22:24 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvm, linux-kernel, kernel-janitors, walter harms
On Thu, 2012-06-28 at 11:07 +0300, Dan Carpenter wrote:
> The "count" variable needs to be signed here because we use it to store
> negative error codes.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Just declare count as signed.
>
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index a4f7321..2e00aa8 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -1419,7 +1419,7 @@ void vfio_config_free(struct vfio_pci_device *vdev)
> }
>
> static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
> - size_t count, loff_t *ppos, bool iswrite)
> + ssize_t count, loff_t *ppos, bool iswrite)
> {
> struct pci_dev *pdev = vdev->pdev;
> struct perm_bits *perm;
signed doesn't seem right for count since just below this chunk we do:
if (*ppos < 0 || *ppos + count > pdev->cfg_size)
return -EFAULT;
So then we have to start testing for negative count. I've added a
ssize_t variable for return that should clear things up. Thanks for the
report!
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [patch 1/3] vfio: signedness bug in vfio_config_do_rw()
2012-06-28 6:44 ` Dan Carpenter
(?)
(?)
@ 2012-06-28 8:05 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 8:05 UTC (permalink / raw)
To: kernel-janitors
On Thu, Jun 28, 2012 at 09:15:12AM +0200, walter harms wrote:
>
>
> Am 28.06.2012 08:44, schrieb Dan Carpenter:
> > The "count" variable is unsigned here so the test for errors doesn't
> > work.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> > index a4f7321..10bc6a8 100644
> > --- a/drivers/vfio/pci/vfio_pci_config.c
> > +++ b/drivers/vfio/pci/vfio_pci_config.c
> > @@ -1487,7 +1487,7 @@ static ssize_t vfio_config_do_rw(struct vfio_pci_device *vdev, char __user *buf,
> > if (perm->readfn) {
> > count = perm->readfn(vdev, *ppos, count,
> > perm, offset, &val);
> > - if (count < 0)
> > + if ((ssize_t)count < 0)
> > return count;
> > }
> >
>
> hi,
> I can only find a few references to vfio_config_do_rw(). From the
> patchit seems to me this is a wrapper for perm->readfn since both
> return ssize_t so why i count unsigned in the first place ?
Yeah. You're right. That was stupid of me. I'll resend.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 32+ messages in thread
* [patch 2/3] vfio: make count unsigned to prevent integer underflow
2012-06-26 4:55 ` Alex Williamson
@ 2012-06-28 6:44 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 6:44 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, linux-kernel, kernel-janitors
In vfio_pci_ioctl() there is a potential integer underflow where we
might allocate less data than intended. We check that hdr.count is not
too large, but we don't check whether it is negative:
drivers/vfio/pci/vfio_pci.c
312 if (hdr.argsz - minsz < hdr.count * size ||
313 hdr.count > vfio_pci_get_irq_count(vdev, hdr.index))
314 return -EINVAL;
315
316 data = kmalloc(hdr.count * size, GFP_KERNEL);
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 300d49b..86ef2da 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -347,7 +347,7 @@ struct vfio_irq_set {
#define VFIO_IRQ_SET_ACTION_TRIGGER (1 << 5) /* Trigger interrupt */
__u32 index;
__s32 start;
- __s32 count;
+ __u32 count;
__u8 data[];
};
#define VFIO_DEVICE_SET_IRQS _IO(VFIO_TYPE, VFIO_BASE + 10)
^ permalink raw reply related [flat|nested] 32+ messages in thread* [patch 2/3] vfio: make count unsigned to prevent integer underflow
@ 2012-06-28 6:44 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 6:44 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, linux-kernel, kernel-janitors
In vfio_pci_ioctl() there is a potential integer underflow where we
might allocate less data than intended. We check that hdr.count is not
too large, but we don't check whether it is negative:
drivers/vfio/pci/vfio_pci.c
312 if (hdr.argsz - minsz < hdr.count * size ||
313 hdr.count > vfio_pci_get_irq_count(vdev, hdr.index))
314 return -EINVAL;
315
316 data = kmalloc(hdr.count * size, GFP_KERNEL);
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 300d49b..86ef2da 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -347,7 +347,7 @@ struct vfio_irq_set {
#define VFIO_IRQ_SET_ACTION_TRIGGER (1 << 5) /* Trigger interrupt */
__u32 index;
__s32 start;
- __s32 count;
+ __u32 count;
__u8 data[];
};
#define VFIO_DEVICE_SET_IRQS _IO(VFIO_TYPE, VFIO_BASE + 10)
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [patch 2/3] vfio: make count unsigned to prevent integer underflow
2012-06-28 6:44 ` Dan Carpenter
@ 2012-06-28 22:24 ` Alex Williamson
-1 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-28 22:24 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvm, linux-kernel, kernel-janitors
On Thu, 2012-06-28 at 09:44 +0300, Dan Carpenter wrote:
> In vfio_pci_ioctl() there is a potential integer underflow where we
> might allocate less data than intended. We check that hdr.count is not
> too large, but we don't check whether it is negative:
>
> drivers/vfio/pci/vfio_pci.c
> 312 if (hdr.argsz - minsz < hdr.count * size ||
> 313 hdr.count > vfio_pci_get_irq_count(vdev, hdr.index))
> 314 return -EINVAL;
> 315
> 316 data = kmalloc(hdr.count * size, GFP_KERNEL);
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 300d49b..86ef2da 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -347,7 +347,7 @@ struct vfio_irq_set {
> #define VFIO_IRQ_SET_ACTION_TRIGGER (1 << 5) /* Trigger interrupt */
> __u32 index;
> __s32 start;
> - __s32 count;
> + __u32 count;
> __u8 data[];
> };
> #define VFIO_DEVICE_SET_IRQS _IO(VFIO_TYPE, VFIO_BASE + 10)
Good find. I've actually trickled this through to change a number of
the function params to unsigned from int. Also in this struct, start
should be unsigned. Thanks for the report!
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [patch 2/3] vfio: make count unsigned to prevent integer underflow
@ 2012-06-28 22:24 ` Alex Williamson
0 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-28 22:24 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvm, linux-kernel, kernel-janitors
On Thu, 2012-06-28 at 09:44 +0300, Dan Carpenter wrote:
> In vfio_pci_ioctl() there is a potential integer underflow where we
> might allocate less data than intended. We check that hdr.count is not
> too large, but we don't check whether it is negative:
>
> drivers/vfio/pci/vfio_pci.c
> 312 if (hdr.argsz - minsz < hdr.count * size ||
> 313 hdr.count > vfio_pci_get_irq_count(vdev, hdr.index))
> 314 return -EINVAL;
> 315
> 316 data = kmalloc(hdr.count * size, GFP_KERNEL);
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 300d49b..86ef2da 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -347,7 +347,7 @@ struct vfio_irq_set {
> #define VFIO_IRQ_SET_ACTION_TRIGGER (1 << 5) /* Trigger interrupt */
> __u32 index;
> __s32 start;
> - __s32 count;
> + __u32 count;
> __u8 data[];
> };
> #define VFIO_DEVICE_SET_IRQS _IO(VFIO_TYPE, VFIO_BASE + 10)
Good find. I've actually trickled this through to change a number of
the function params to unsigned from int. Also in this struct, start
should be unsigned. Thanks for the report!
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* [patch 3/3] vfio: return -EFAULT on failure
2012-06-26 4:55 ` Alex Williamson
@ 2012-06-28 6:45 ` Dan Carpenter
-1 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 6:45 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, linux-kernel, kernel-janitors
This ioctl function is supposed to return a negative error code or zero
on success. copy_to_user() returns zero or the number of bytes
remaining to be copied.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 457acf3..1aa373f 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1159,6 +1159,8 @@ static long vfio_group_fops_unl_ioctl(struct file *filep,
status.flags |= VFIO_GROUP_FLAGS_CONTAINER_SET;
ret = copy_to_user((void __user *)arg, &status, minsz);
+ if (ret)
+ ret = -EFAULT;
break;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [patch 3/3] vfio: return -EFAULT on failure
@ 2012-06-28 6:45 ` Dan Carpenter
0 siblings, 0 replies; 32+ messages in thread
From: Dan Carpenter @ 2012-06-28 6:45 UTC (permalink / raw)
To: Alex Williamson; +Cc: kvm, linux-kernel, kernel-janitors
This ioctl function is supposed to return a negative error code or zero
on success. copy_to_user() returns zero or the number of bytes
remaining to be copied.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 457acf3..1aa373f 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1159,6 +1159,8 @@ static long vfio_group_fops_unl_ioctl(struct file *filep,
status.flags |= VFIO_GROUP_FLAGS_CONTAINER_SET;
ret = copy_to_user((void __user *)arg, &status, minsz);
+ if (ret)
+ ret = -EFAULT;
break;
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [patch 3/3] vfio: return -EFAULT on failure
2012-06-28 6:45 ` Dan Carpenter
@ 2012-06-28 22:25 ` Alex Williamson
-1 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-28 22:25 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvm, linux-kernel, kernel-janitors
On Thu, 2012-06-28 at 09:45 +0300, Dan Carpenter wrote:
> This ioctl function is supposed to return a negative error code or zero
> on success. copy_to_user() returns zero or the number of bytes
> remaining to be copied.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 457acf3..1aa373f 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1159,6 +1159,8 @@ static long vfio_group_fops_unl_ioctl(struct file *filep,
> status.flags |= VFIO_GROUP_FLAGS_CONTAINER_SET;
>
> ret = copy_to_user((void __user *)arg, &status, minsz);
> + if (ret)
> + ret = -EFAULT;
>
> break;
> }
Yes, thank you! I've folded all of these into the commits on my next
branch, so they should be cleaned up in tomorrow's tree. Thanks for the
reports, please let me know if you find more.
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [patch 3/3] vfio: return -EFAULT on failure
@ 2012-06-28 22:25 ` Alex Williamson
0 siblings, 0 replies; 32+ messages in thread
From: Alex Williamson @ 2012-06-28 22:25 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvm, linux-kernel, kernel-janitors
On Thu, 2012-06-28 at 09:45 +0300, Dan Carpenter wrote:
> This ioctl function is supposed to return a negative error code or zero
> on success. copy_to_user() returns zero or the number of bytes
> remaining to be copied.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 457acf3..1aa373f 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1159,6 +1159,8 @@ static long vfio_group_fops_unl_ioctl(struct file *filep,
> status.flags |= VFIO_GROUP_FLAGS_CONTAINER_SET;
>
> ret = copy_to_user((void __user *)arg, &status, minsz);
> + if (ret)
> + ret = -EFAULT;
>
> break;
> }
Yes, thank you! I've folded all of these into the commits on my next
branch, so they should be cleaned up in tomorrow's tree. Thanks for the
reports, please let me know if you find more.
Alex
^ permalink raw reply [flat|nested] 32+ messages in thread