Re: [Qemu-devel] [PATCH v2 1/6] qerror: add MAX_KEYCODES 16

[Amos Kong]

On 18/06/12 23:30, Amos Kong wrote:
> On 06/15/2012 09:35 PM, Luiz Capitulino wrote:
> > On Fri, 15 Jun 2012 09:57:49 +0200
> > Gerd Hoffmann<address@hidden>  wrote:
> >
> > >    Hi,
> > >
> > > > > It seems we need to notice user when inputted keys are more than 16.
> > > >
> > > > Hi Gerd,
> > > >
> > > > When I use 'sendkey' command to send key-series to guest, some keyboard
> > > > events will be send. There is a limitation (16) that was introduced by this
> > > > old commit c8256f9d (without description). Do you know the reason?
> > >
> > > Probably hardware limitation, ps/2 keyboards can buffer up to 16 keys IIRC.
> >
> > Then the perfect thing to do would be to drop the MAX_KEYCODES check from
> > the sendkey command and move bounds checking down to the device emulation code.
> >
> >
> > However, this will require a bit of code churn if we do it for all devices,
> > and won't buy us much, as the most likely reason for the error is a client/user
> > trying to send too many keys in parallel to the guest, right?
>
> Agree, we can notice in stderr when the redundant keys are ignored as hid.
>
>
> #define QUEUE_LENGTH    16 /* should be enough for a triple-click */
>
> static void hid_keyboard_event(void *opaque, int keycode)
> {
>      ...
>      if (hs->n == QUEUE_LENGTH) {
>          fprintf(stderr, "usb-kbd: warning: key event queue full\n");
>          return;
>      }


I dropped the limitation in sendkey command,
and didn't change current ps2.c, executed some
tests in different environments.

environment             max inputted key number
------------            -----------------------
win7 notepad            100
rhel6 grub              15
rhel6 pxe               15
rhel6 login window      10
rhel6 vim               16
rhel6 terminal(init 3)  200


It seems original 256 queue limitation in ps2.c is fine.
I would only drop limitation(16) in old sendkey command,
it's secure.


> > If this is right, then I think that the best thing to do would be to drop the
> > MAX_KEYCODES check from the sendkey command and document that devices can drop
> > keys if too many of them are sent in parallel or too fast (we can mention ps/2
> > as an example of a 16 bytes limit).
> >
> > > Likewise the usb hid devices can buffer up to 16 events.  In that case
> > > it is just a qemu implementation detail and not a property of the
> > > hardware we are emulating, so it can be changed.  Not trivially though
> > > as the buffer is part of the migration data, so it is more work that
> > > just changing a #define.


--
                        Amos.