Re: [PATCH v2 3/6] kvm: Sanitize KVM_IRQFD flags

2012-06-28 Thread Avi Kivity 
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 alex.william...@redhat.com
 
 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

2012-06-27 Thread Michael S. Tsirkin 
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 alex.william...@redhat.com

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

Re: [PATCH v2 3/6] kvm: Sanitize KVM_IRQFD flags

2012-06-27 Thread Alex Williamson 
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 alex.william...@redhat.com
 
 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

2012-06-27 Thread Michael S. Tsirkin 
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 alex.william...@redhat.com
  
  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

[PATCH v2 3/6] kvm: Sanitize KVM_IRQFD flags

2012-06-26 Thread Alex Williamson 
We only know of one so far.

Signed-off-by: Alex Williamson alex.william...@redhat.com
---

 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