Re: [PATCH v3 1/2] drm/udl: reject short USB control transfers in udl_read_edid_block()

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 24.09.26 um 09:30 schrieb Hui Peng:
> In udl_read_edid_block(), usb_control_msg() requests 2 bytes into
> read_buff for each EDID byte, and read_buff[1] is stored into buf[i].
> Currently the short-transfer check tests ret < 1 instead of ret < 2, so
> if a short USB control transfer returns 1 byte, read_buff[1] is not
> written by usb_control_msg() and retains the previous iteration's byte (or
> uninitialized kmalloc(2) memory on the first iteration), which is then
> copied into buf[i].
>
> Change the check from ret < 1 to ret < 2 so any short USB control
> transfer fails with -EIO.
>
> Tested in QEMU against Linux 7.3.0-rc3 using dummy_hcd and raw-gadget
> emulating a DisplayLink UDL USB device: on the unfixed kernel, when
> usb_control_msg() returned 1 byte for an EDID read (short transfer),
> udl_read_edid_block() accepted ret = 1 and stored stale read_buff[1]
> data into the EDID buffer; whereas with this fix applied, ret < 2
> correctly fails with -EIO and prevents EDID buffer corruption.
>
> Fixes: 5320918b9a87 ("drm/udl: initial UDL driver (v4)")
> Cc: stable@vger.kernel.org
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks for the fix.

Best regards
Thomas

> ---
> Changes in v3:
> - Change (ret != 2) to (ret < 2) in udl_read_edid_block(), as requested
>    by Thomas Zimmermann.
>
> Changes in v2:
> - Split out as patch 1/2 as requested by Jani Nikula.
> - Added testing details in QEMU on short USB EDID control transfers.
>
>   drivers/gpu/drm/udl/udl_edid.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_edid.c b/drivers/gpu/drm/udl/udl_edid.c
> index af4cff2a7c51..2120e363b6eb 100644
> --- a/drivers/gpu/drm/udl/udl_edid.c
> +++ b/drivers/gpu/drm/udl/udl_edid.c
> @@ -36,7 +36,7 @@ static int udl_read_edid_block(void *data, u8 *buf, unsigned int block, size_t l
>   		if (ret < 0) {
>   			drm_err(dev, "Read EDID byte %zu failed err %x\n", i, ret);
>   			goto err_drm_dev_exit;
> -		} else if (ret < 1) {
> +		} else if (ret < 2) {
>   			ret = -EIO;
>   			drm_err(dev, "Read EDID byte %zu failed\n", i);
>   			goto err_drm_dev_exit;

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)