Hi
On Tue, Nov 26, 2013 at 8:02 AM, Ben Hutchings <ben@xxxxxxxxxxxxxxx> wrote:
> Short event writes are normally padded with zeroes, but the compat
> fixup for UHID_CREATE didn't ensure this. This appears to allow an
> information leak.
>
> Compile-tested only.
>
> Fixes: befde0226a59 ('HID: uhid: make creating devices work on 64/32 systems')
> Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> I have no familiarity with uhid so I haven't written a test for this.
> It looks like it would be possible to write a UHID_CREATE event that
> only covers fields up to rd_size, and the following data on the heap
> would be copied to the HID device metadata and be readable that way.
>
> Ben.
>
> drivers/hid/uhid.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 5bf2fb7..579a7115 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -298,6 +298,9 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
> kfree(compat);
> return -EFAULT;
> }
> + if (len < sizeof(*compat))
> + memset((char *)buffer + len, 0,
> + sizeof(*compat) - len);
This should be "compat", not "buffer".
Anyhow, nice catch! But the better fix imho is to use kzalloc() for
the "compat" object. This isn't performance-critical and we can avoid
any other off-by-one bug or future conversion errors. And besides,
it's far easier to read than this memset().
Thanks
David
> /* Shuffle the data over to proper structure */
> event->type = type;
>
> --
> Ben Hutchings
> Usenet is essentially a HUGE group of people passing notes in class.
> - Rachel Kadel, `A Quick Guide to Newsgroup Etiquette'
--
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
