Re: [PATCH] drivers/hid: fix for the big hid report length

[Sabyrzhan Tasbolatov]

On Thu, 25 Feb 2021 10:59:14 -0500, Alan Stern wrote:
> Won't this cause silent errors?

Agree. But there are already such as cases like in:

// net/bluetooth/hidp/core.c
static void hidp_process_report(..)
{
	..
	if (len > HID_MAX_BUFFER_SIZE)
		len = HID_MAX_BUFFER_SIZE;
	..

// drivers/hid/hid-core.c
int hid_report_raw_event(..)
{
	..
	rsize = hid_compute_report_size(report);

	if (report_enum->numbered && rsize >= HID_MAX_BUFFER_SIZE)
		rsize = HID_MAX_BUFFER_SIZE - 1;
	else if (rsize > HID_MAX_BUFFER_SIZE)
		rsize = HID_MAX_BUFFER_SIZE;
	..

// drivers/staging/greybus/hid.c
static int gb_hid_start(..)
{
	..
	if (bufsize > HID_MAX_BUFFER_SIZE)
		bufsize = HID_MAX_BUFFER_SIZE;
	..

> How about instead just rejecting any device which includes a report 
> whose length is too big (along with an line in the system log explaining 
> what's wrong)?

Not everywhere, but there are already such as logs when > HID_MAX_BUFFER_SIZE

// drivers/hid/hidraw.c
static ssize_t hidraw_send_report(..)
{
	..
	if (count > HID_MAX_BUFFER_SIZE) {
		hid_warn(dev, "pid %d passed too large report\n",
			 task_pid_nr(current));
		ret = -EINVAL;
		goto out;
	}


I've just noticed that hid_compute_report_size() doing the same thing as
hid_report_len(). So I will replace it with latter one with length check.

So in [PATCH v2] I will do following:

 1. replace hid_compute_report_size() with hid_report_len()

 2. in hid_report_len() we can hid_warn() if length > HID_MAX_BUFFER_SIZE,
and return HID_MAX_BUFFER_SIZE. Or we can return 0 in hid_report_len() to let
functions like hid_hw_raw_request(), hid_hw_output_report() to validate
invalid report length and return -EINVAL. Though I'll need to add !length
missing checks in other places.

Please let me know what it's preferred way in 2nd step.