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.
