Re: [PATCH] HID: uhid: Fix sending events with invalid data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Vinicius

CC'ing Jiri

On Sat, Jul 14, 2012 at 11:59 PM, Vinicius Costa Gomes
<vinicius.gomes@xxxxxxxxxxxxx> wrote:
> This was detected because events with invalid types were arriving
> to userspace.
>
> The code before this patch would only work for the first event in the
> queue (when uhid->tail is 0).
>
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@xxxxxxxxxxxxx>
> ---
> David,
>
> Do you have any clue why it took so long for us to detect this issue?

Ugh! Thanks for spotting this! I actually have no idea why this wasn't
detected earlier. I always run the sample file in ./samples/uhid/ and
they worked fine. Could it be that this only occurs when the queue
runs full? That is, there is more than one event in the queue? This
would explain why I never saw this bug. Or maybe the first event is
just repeated all the time instead of reporting the new events which
would explain why the example in ./samples/uhid works fine.

It definitely has to do with the late transition from a fixed array to
a dynamically allocated pointer-array so all the early tests from the
bluez guys worked fine. This is probably also the reason why we all
didn't encounter this in the early tests.

I just recompiled the example with the old code and it still works
fine. I occasionally see the "invalid event" warnings now, but only 1
or 2 times. Maybe it's just dropping all except the 32'th package
(buffer-size) so I cannot see it and the mouse-emulation still works
smoothly (but at 1/32th of the original speed). Anyway, the patch
looks good, thanks!

Reviewed-by: David Herrmann <dh.herrmann@xxxxxxxxxxxxxx>
Thanks!

David

> Cheers,
>
>
>  drivers/hid/uhid.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 119b7e6..714cd8c 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -465,7 +465,7 @@ try_again:
>                 goto try_again;
>         } else {
>                 len = min(count, sizeof(**uhid->outq));
> -               if (copy_to_user(buffer, &uhid->outq[uhid->tail], len)) {
> +               if (copy_to_user(buffer, uhid->outq[uhid->tail], len)) {
>                         ret = -EFAULT;
>                 } else {
>                         kfree(uhid->outq[uhid->tail]);
> --
> 1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux