2.6.32-KVM-pit_ioport_read() integer buffer overflow hole

2.6.32-KVM-pit_ioport_read() integer buffer overflow hole

[wzt wzt]

Hi:
        In kernel 2.6.32 kernel/arch/x86/kvm/i8254.c， I found
pit_ioport_read maybe have a integer buffer overflow hole:

static int pit_ioport_read(struct kvm_io_device *this,
                          gpa_t addr, int len, void *data)
{
…
       if (len > sizeof(ret))
               len = sizeof(ret);

       memcpy(data, (char *)&ret, len);  // if len is a negative(< 0),
 the data memory will be buffer overflow.
…
}

static const struct kvm_io_device_ops pit_dev_ops = {
       .read     = pit_ioport_read,
       .write    = pit_ioport_write,
};

The same bug also in speaker_ioport_read() function:

static int speaker_ioport_read(struct kvm_io_device *this,
                              gpa_t addr, int len, void *data)
{
…
       if (len > sizeof(ret))
               len = sizeof(ret);
       memcpy(data, (char *)&ret, len);
…
}

static const struct kvm_io_device_ops speaker_dev_ops = {
       .read     = speaker_ioport_read,
       .write    = speaker_ioport_write,
};

My patch is:

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 296aba4..bf8637f 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -463,6 +463,8 @@ static int pit_ioport_read(struct kvm_io_device *this,
       struct kvm *kvm = pit->kvm;
       int ret, count;
       struct kvm_kpit_channel_state *s;
+       if (len < 0)
+               return -EOPNOTSUPP;
       if (!pit_in_range(addr))
               return -EOPNOTSUPP;

@@ -516,6 +518,7 @@ static int pit_ioport_read(struct kvm_io_device *this,

       if (len > sizeof(ret))
               len = sizeof(ret);
+
       memcpy(data, (char *)&ret, len);

       mutex_unlock(&pit_state->lock);
@@ -547,6 +550,9 @@ static int speaker_ioport_read(struct kvm_io_device *this,
       struct kvm *kvm = pit->kvm;
       unsigned int refresh_clock;
       int ret;
+
+       if (len < 0)
+               return -EOPNOTSUPP;
       if (addr != KVM_SPEAKER_BASE_ADDRESS)
               return -EOPNOTSUPP;

Re: 2.6.32-KVM-pit_ioport_read() integer buffer overflow hole

[Avi Kivity]

On 01/26/2010 10:59 AM, wzt wzt wrote:
> Hi:
>          In kernel 2.6.32 kernel/arch/x86/kvm/i8254.c， I found
> pit_ioport_read maybe have a integer buffer overflow hole:
>
> static int pit_ioport_read(struct kvm_io_device *this,
>                            gpa_t addr, int len, void *data)
> {
> …
>         if (len>  sizeof(ret))
>                 len = sizeof(ret);
>
>         memcpy(data, (char *)&ret, len);  // if len is a negative(<  0),
>   the data memory will be buffer overflow.
> …
> }
>    


Is there any caller that can send a negative length, user- or guest- 
controlled?

-- 
error compiling committee.c: too many arguments to function