Re: [PATCH v2 3/6] kvm: Sanitize KVM_IRQFD flags
On 06/27/2012 12:21 PM, Michael S. Tsirkin wrote: > On Tue, Jun 26, 2012 at 11:09:32PM -0600, Alex Williamson wrote: >> We only know of one so far. >> >> Signed-off-by: Alex Williamson > > Ugh. So we have a bug: we should have sanitized the fields. > If there's buggy userspace that only set the low bit > it will break with this change. > Is it too late now? Do we need KVM_IRQFD2 which > sanitized fields properly? Avi? We try and see. Commit this, if somebody complain, revert after apologizing profusely. If no one notices, we can claim those bits. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] kvm: Sanitize KVM_IRQFD flags
On Wed, Jun 27, 2012 at 02:12:18PM -0600, Alex Williamson wrote:
> On Wed, 2012-06-27 at 12:21 +0300, Michael S. Tsirkin wrote:
> > On Tue, Jun 26, 2012 at 11:09:32PM -0600, Alex Williamson wrote:
> > > We only know of one so far.
> > >
> > > Signed-off-by: Alex Williamson
> >
> > Ugh. So we have a bug: we should have sanitized the fields.
> > If there's buggy userspace that only set the low bit
> > it will break with this change.
> > Is it too late now? Do we need KVM_IRQFD2 which
> > sanitized fields properly? Avi?
>
> If we take that attitude that we haven't sanitized the bits in the past
> and therefore all other bits are tainted from future use, we might as
> well toss out this ioctl and start over. There's no way to add
> anything. :-\
This is what I'm asking.
> > > ---
> > >
> > > virt/kvm/eventfd.c |3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > > index c307c24..7d7e2aa 100644
> > > --- a/virt/kvm/eventfd.c
> > > +++ b/virt/kvm/eventfd.c
> > > @@ -340,6 +340,9 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
> > > *args)
> > > int
> > > kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
> > > {
> > > + if (args->flags & ~KVM_IRQFD_FLAG_DEASSIGN)
> > > + return -EINVAL;
> > > +
> > > if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
> > > return kvm_irqfd_deassign(kvm, args);
> > >
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] kvm: Sanitize KVM_IRQFD flags
On Wed, 2012-06-27 at 12:21 +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 26, 2012 at 11:09:32PM -0600, Alex Williamson wrote:
> > We only know of one so far.
> >
> > Signed-off-by: Alex Williamson
>
> Ugh. So we have a bug: we should have sanitized the fields.
> If there's buggy userspace that only set the low bit
> it will break with this change.
> Is it too late now? Do we need KVM_IRQFD2 which
> sanitized fields properly? Avi?
If we take that attitude that we haven't sanitized the bits in the past
and therefore all other bits are tainted from future use, we might as
well toss out this ioctl and start over. There's no way to add
anything. :-\
> > ---
> >
> > virt/kvm/eventfd.c |3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> > index c307c24..7d7e2aa 100644
> > --- a/virt/kvm/eventfd.c
> > +++ b/virt/kvm/eventfd.c
> > @@ -340,6 +340,9 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
> > *args)
> > int
> > kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
> > {
> > + if (args->flags & ~KVM_IRQFD_FLAG_DEASSIGN)
> > + return -EINVAL;
> > +
> > if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
> > return kvm_irqfd_deassign(kvm, args);
> >
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] kvm: Sanitize KVM_IRQFD flags
On Tue, Jun 26, 2012 at 11:09:32PM -0600, Alex Williamson wrote:
> We only know of one so far.
>
> Signed-off-by: Alex Williamson
Ugh. So we have a bug: we should have sanitized the fields.
If there's buggy userspace that only set the low bit
it will break with this change.
Is it too late now? Do we need KVM_IRQFD2 which
sanitized fields properly? Avi?
> ---
>
> virt/kvm/eventfd.c |3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
> index c307c24..7d7e2aa 100644
> --- a/virt/kvm/eventfd.c
> +++ b/virt/kvm/eventfd.c
> @@ -340,6 +340,9 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd
> *args)
> int
> kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
> {
> + if (args->flags & ~KVM_IRQFD_FLAG_DEASSIGN)
> + return -EINVAL;
> +
> if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
> return kvm_irqfd_deassign(kvm, args);
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/6] kvm: Sanitize KVM_IRQFD flags
We only know of one so far.
Signed-off-by: Alex Williamson
---
virt/kvm/eventfd.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index c307c24..7d7e2aa 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -340,6 +340,9 @@ kvm_irqfd_deassign(struct kvm *kvm, struct kvm_irqfd *args)
int
kvm_irqfd(struct kvm *kvm, struct kvm_irqfd *args)
{
+ if (args->flags & ~KVM_IRQFD_FLAG_DEASSIGN)
+ return -EINVAL;
+
if (args->flags & KVM_IRQFD_FLAG_DEASSIGN)
return kvm_irqfd_deassign(kvm, args);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
