Re: [PATCH v2 2/2] HID: logitech-dj: Prevent REPORT_ID_DJ_SHORT related user initiated OOB write

[Benjamin Tissoires <bentiss@kernel.org>]

On Mar 24 2026, Lee Jones wrote:
> logi_dj_recv_send_report() assumes that all incoming REPORT_ID_DJ_SHORT
> reports are 14 Bytes (DJREPORT_SHORT_LENGTH - 1) long.  It uses that
> assumption to load the associated field's 'value' array with 14 Bytes of
> data.  However, if a malicious user only sends say 1 Byte of data,
> 'report_count' will be 1 and only 1 Byte of memory will be allocated to
> the 'value' Byte array.  When we come to populate 'value[1-13]' we will
> experience an OOB write.
> 
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
> v1 => v2: Move handling to .probe()
> 
>  drivers/hid/hid-logitech-dj.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 32139b2561c0..a8082199d13d 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -1859,6 +1859,7 @@ static int logi_dj_probe(struct hid_device *hdev,
>  			 const struct hid_device_id *id)
>  {
>  	struct hid_report_enum *input_report_enum;
> +	struct hid_report_enum *output_report_enum;
>  	struct hid_report *rep;
>  	struct dj_receiver_dev *djrcv_dev;
>  	struct usb_interface *intf;
> @@ -1903,6 +1904,15 @@ static int logi_dj_probe(struct hid_device *hdev,
>  		}
>  	}
>  
> +	output_report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
> +	rep = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];

We've got an issue here: the driver binds on several HID devices that
can have no REPORT_ID_DJ_SHORT in the output reports. On those devices
(like the mouse/keyboard emulation on the receiver itself), rep is null.

And of course this segfaults in the test below.

A simple "if (rep &&)" should solve the issue:

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 838c6de9a921..7c09faedefbd 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1907,7 +1907,7 @@ static int logi_dj_probe(struct hid_device *hdev,
        output_report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
        rep = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
 
-       if (rep->maxfield < 1 || rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1) {
+       if (rep && (rep->maxfield < 1 || rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1)) {
                hid_err(hdev, "Expected size of DJ short report is %d, but got %d",
                        DJREPORT_SHORT_LENGTH - 1, rep->field[0]->report_count);
                return -EINVAL;

Cheers,
Benjamin

> +
> +	if (rep->maxfield < 1 || rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1) {
> +		hid_err(hdev, "Expected size of DJ short report is %d, but got %d",
> +			DJREPORT_SHORT_LENGTH - 1, rep->field[0]->report_count);
> +		return -EINVAL;
> +	}
> +
>  	input_report_enum = &hdev->report_enum[HID_INPUT_REPORT];
>  
>  	/* no input reports, bail out */
> -- 
> 2.53.0.983.g0bb29b3bc5-goog
> 
>