Commit 2c85c61d authored by Benjamin Tissoires's avatar Benjamin Tissoires Committed by Jiri Kosina
Browse files

HID: pass the buffer size to hid_report_raw_event 



commit 0a3fe972 ("HID: core: Mitigate potential OOB by removing
bogus memset()") enforced the provided data to be at least the size of
the declared buffer in the report descriptor to prevent a buffer
overflow. However, we can try to be smarter by providing both the buffer
size and the data size, meaning that hid_report_raw_event() can make
better decision whether we should plaining reject the buffer (buffer
overflow attempt) or if we can safely memset it to 0 and pass it to the
rest of the stack.

Fixes: 0a3fe972 ("HID: core: Mitigate potential OOB by removing bogus memset()")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarBenjamin Tissoires <bentiss@kernel.org>
Acked-by: default avatarJohan Hovold <johan@kernel.org>
Reviewed-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.com>
parent b08665fe

drivers/hid/bpf/hid_bpf_dispatch.c

+4 −2
Original line number Diff line number Diff line
@@ -24,7 +24,8 @@ EXPORT_SYMBOL(hid_ops); 


u8 *

dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type, u8 *data,

			      u32 *size, int interrupt, u64 source, bool from_bpf)

			      size_t *buf_size, u32 *size, int interrupt, u64 source,

			      bool from_bpf)

{

	struct hid_bpf_ctx_kern ctx_kern = {

		.ctx = {
@@ -74,6 +75,7 @@ dispatch_hid_bpf_device_event(struct hid_device *hdev, enum hid_report_type type 
		*size = ret;

	}



	*buf_size = ctx_kern.ctx.allocated_size;

	return ctx_kern.data;

}

EXPORT_SYMBOL_GPL(dispatch_hid_bpf_device_event);
@@ -505,7 +507,7 @@ __hid_bpf_input_report(struct hid_bpf_ctx *ctx, enum hid_report_type type, u8 *b 
	if (ret)

		return ret;



	return hid_ops->hid_input_report(ctx->hid, type, buf, size, 0, (u64)(long)ctx, true,

	return hid_ops->hid_input_report(ctx->hid, type, buf, size, size, 0, (u64)(long)ctx, true,

					 lock_already_taken);

}

drivers/hid/hid-core.c

+28 −14
Original line number Diff line number Diff line
@@ -2033,24 +2033,32 @@ int __hid_request(struct hid_device *hid, struct hid_report *report, 
}

EXPORT_SYMBOL_GPL(__hid_request);



int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,

			 int interrupt)

int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *data,

			 size_t bufsize, u32 size, int interrupt)

{

	struct hid_report_enum *report_enum = hid->report_enum + type;

	struct hid_report *report;

	struct hid_driver *hdrv;

	int max_buffer_size = HID_MAX_BUFFER_SIZE;

	u32 rsize, csize = size;

	size_t bsize = bufsize;

	u8 *cdata = data;

	int ret = 0;



	report = hid_get_report(report_enum, data);

	if (!report)

		goto out;

		return 0;



	if (unlikely(bsize < csize)) {

		hid_warn_ratelimited(hid, "Event data for report %d is incorrect (%d vs %ld)\n",

				     report->id, csize, bsize);

		return -EINVAL;

	}



	if (report_enum->numbered) {

		cdata++;

		csize--;

		bsize--;

	}



	rsize = hid_compute_report_size(report);
@@ -2063,11 +2071,16 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 * 
	else if (rsize > max_buffer_size)

		rsize = max_buffer_size;



	if (bsize < rsize) {

		hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %ld)\n",

				     report->id, rsize, bsize);

		return -EINVAL;

	}



	if (csize < rsize) {

		hid_warn_ratelimited(hid, "Event data for report %d was too short (%d vs %d)\n",

				     report->id, rsize, csize);

		ret = -EINVAL;

		goto out;

		dbg_hid("report %d is too short, (%d < %d)\n", report->id,

			csize, rsize);

		memset(cdata + csize, 0, rsize - csize);

	}



	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
@@ -2075,7 +2088,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 * 
	if (hid->claimed & HID_CLAIMED_HIDRAW) {

		ret = hidraw_report_event(hid, data, size);

		if (ret)

			goto out;

			return ret;

	}



	if (hid->claimed != HID_CLAIMED_HIDRAW && report->maxfield) {
@@ -2087,15 +2100,15 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 * 


	if (hid->claimed & HID_CLAIMED_INPUT)

		hidinput_report_event(hid, report);

out:



	return ret;

}

EXPORT_SYMBOL_GPL(hid_report_raw_event);





static int __hid_input_report(struct hid_device *hid, enum hid_report_type type,

			      u8 *data, u32 size, int interrupt, u64 source, bool from_bpf,

			      bool lock_already_taken)

			      u8 *data, size_t bufsize, u32 size, int interrupt, u64 source,

			      bool from_bpf, bool lock_already_taken)

{

	struct hid_report_enum *report_enum;

	struct hid_driver *hdrv;
@@ -2120,7 +2133,8 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type, 
	report_enum = hid->report_enum + type;

	hdrv = hid->driver;



	data = dispatch_hid_bpf_device_event(hid, type, data, &size, interrupt, source, from_bpf);

	data = dispatch_hid_bpf_device_event(hid, type, data, &bufsize, &size, interrupt,

					     source, from_bpf);

	if (IS_ERR(data)) {

		ret = PTR_ERR(data);

		goto unlock;
@@ -2149,7 +2163,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type, 
			goto unlock;

	}



	ret = hid_report_raw_event(hid, type, data, size, interrupt);

	ret = hid_report_raw_event(hid, type, data, bufsize, size, interrupt);



unlock:

	if (!lock_already_taken)
@@ -2171,7 +2185,7 @@ static int __hid_input_report(struct hid_device *hid, enum hid_report_type type, 
int hid_input_report(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 size,

		     int interrupt)

{

	return __hid_input_report(hid, type, data, size, interrupt, 0,

	return __hid_input_report(hid, type, data, size, size, interrupt, 0,

				  false, /* from_bpf */

				  false /* lock_already_taken */);

}

drivers/hid/hid-gfrm.c

+2 −2
Original line number Diff line number Diff line
@@ -66,7 +66,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report, 
	switch (data[1]) {

	case GFRM100_SEARCH_KEY_DOWN:

		ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_dn,

					   sizeof(search_key_dn), 1);

					   sizeof(search_key_dn), sizeof(search_key_dn), 1);

		break;



	case GFRM100_SEARCH_KEY_AUDIO_DATA:
@@ -74,7 +74,7 @@ static int gfrm_raw_event(struct hid_device *hdev, struct hid_report *report, 


	case GFRM100_SEARCH_KEY_UP:

		ret = hid_report_raw_event(hdev, HID_INPUT_REPORT, search_key_up,

					   sizeof(search_key_up), 1);

					   sizeof(search_key_up), sizeof(search_key_up), 1);

		break;



	default:

drivers/hid/hid-logitech-hidpp.c

+1 −1
Original line number Diff line number Diff line
@@ -3673,7 +3673,7 @@ static int hidpp10_consumer_keys_raw_event(struct hidpp_device *hidpp, 
	memcpy(&consumer_report[1], &data[3], 4);

	/* We are called from atomic context */

	hid_report_raw_event(hidpp->hid_dev, HID_INPUT_REPORT,

			     consumer_report, 5, 1);

			     consumer_report, sizeof(consumer_report), 5, 1);



	return 1;

}

drivers/hid/hid-multitouch.c

+1 −1
Original line number Diff line number Diff line
@@ -533,7 +533,7 @@ static void mt_get_feature(struct hid_device *hdev, struct hid_report *report) 
		}



		ret = hid_report_raw_event(hdev, HID_FEATURE_REPORT, buf,

					   size, 0);

					   size, size, 0);

		if (ret)

			dev_warn(&hdev->dev, "failed to report feature\n");

	}