In drivers/gpu/drm/udl/ (udl_drv.c, udl_edid.c), check that
usb_control_msg() returns the full 2-byte response in
udl_read_edid_block() before copying read_buff[1], and clean up
allocated URBs if udl_Driver initialization fails.
Fixes: 5320918b9a87 ("drm/udl: initial UDL driver (v4)")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 1922988625eb..239a6983f49f 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -97,8 +97,10 @@ static int udl_usb_probe(struct usb_interface *interface,
return PTR_ERR(udl);
r = drm_dev_register(&udl->drm, 0);
- if (r)
+ if (r) {
+ udl_drop_usb(udl);
return r;
+ }
DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
diff --git a/drivers/gpu/drm/udl/udl_edid.c b/drivers/gpu/drm/udl/udl_edid.c
index af4cff2a7c51..b7c462436573 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;
This series fixes two error-handling issues in the udl DRM driver: 1. Reject short USB control transfers (ret != 2) in udl_read_edid_block() with -EIO so uninitialized or stale bytes from read_buff[1] are not copied into the EDID buffer. 2. Call udl_drop_usb(udl) when drm_dev_register() fails in udl_usb_probe() so the allocated URB list and DMA buffers are released. Changes in v2: - Split into two separate patches, as requested by Jani Nikula. Hui Peng (2): drm/udl: reject short USB control transfers in udl_read_edid_block() drm/udl: clean up USB resources if drm_dev_register() fails drivers/gpu/drm/udl/udl_drv.c | 4 +++- drivers/gpu/drm/udl/udl_edid.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) -- 2.55.0.1082.g2b9226bbc0-goog
This series fixes two error-handling issues in the udl DRM driver: 1. Reject short USB control transfers (ret < 2) in udl_read_edid_block() with -EIO so uninitialized or stale bytes from read_buff[1] are not copied into the EDID buffer. 2. Call udl_drop_usb(udl) when drm_dev_register() fails in udl_usb_probe() so the allocated URB list and DMA buffers are released. Changes in v3: - Change (ret != 2) to (ret < 2) in udl_read_edid_block(), as requested by Thomas Zimmermann. - Add Reviewed-by tag from Thomas Zimmermann to both patches. Changes in v2: - Split into two separate patches, as requested by Jani Nikula. Hui Peng (2): drm/udl: reject short USB control transfers in udl_read_edid_block() drm/udl: clean up USB resources if drm_dev_register() fails drivers/gpu/drm/udl/udl_drv.c | 4 +++- drivers/gpu/drm/udl/udl_edid.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-)
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
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
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..b7c462436573 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;
--
2.55.0.1082.g2b9226bbc0-goog
Hi
Am 24.09.26 um 08:44 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
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> 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..b7c462436573 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) {
Please test for (ret < 2) instead. It makes the test a little less
dependent on the earlier calls.
Best regards
Thomas
> 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)
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>
---
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;
--
2.55.0.1082.g2b9226bbc0-goog
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)
In udl_usb_probe(), udl_driver_create() calls udl_init(udl), which
allocates USB URBs and DMA buffers via udl_alloc_urb_list() that are
normally freed by udl_drop_usb() in udl_usb_disconnect(). If
drm_dev_register() fails in udl_usb_probe(), udl_usb_probe() returns
without calling udl_drop_usb(udl), leaking the allocated URB list and DMA
buffers because udl_usb_disconnect() is not called when probe fails.
Call udl_drop_usb(udl) when drm_dev_register() fails in udl_usb_probe().
Tested in QEMU against Linux 7.3.0-rc3 using dummy_hcd by simulating
a registration error in drm_dev_register(): on the unfixed kernel,
udl_usb_probe() returned an error without calling udl_drop_usb(udl),
leaking 8 allocated URBs and DMA buffers; whereas with this fix applied,
udl_drop_usb(udl) is called on error, freeing all URB and DMA
allocations.
Fixes: 5320918b9a87 ("drm/udl: initial UDL driver (v4)")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v2:
- Split out as patch 2/2 as requested by Jani Nikula.
- Added testing details in QEMU on drm_dev_register() failure cleanup.
drivers/gpu/drm/udl/udl_drv.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 1922988625eb..239a6983f49f 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -97,8 +97,10 @@ static int udl_usb_probe(struct usb_interface *interface,
return PTR_ERR(udl);
r = drm_dev_register(&udl->drm, 0);
- if (r)
+ if (r) {
+ udl_drop_usb(udl);
return r;
+ }
DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
Am 24.09.26 um 08:44 schrieb Hui Peng:
> In udl_usb_probe(), udl_driver_create() calls udl_init(udl), which
> allocates USB URBs and DMA buffers via udl_alloc_urb_list() that are
> normally freed by udl_drop_usb() in udl_usb_disconnect(). If
> drm_dev_register() fails in udl_usb_probe(), udl_usb_probe() returns
> without calling udl_drop_usb(udl), leaking the allocated URB list and DMA
> buffers because udl_usb_disconnect() is not called when probe fails.
>
> Call udl_drop_usb(udl) when drm_dev_register() fails in udl_usb_probe().
>
> Tested in QEMU against Linux 7.3.0-rc3 using dummy_hcd by simulating
> a registration error in drm_dev_register(): on the unfixed kernel,
> udl_usb_probe() returned an error without calling udl_drop_usb(udl),
> leaking 8 allocated URBs and DMA buffers; whereas with this fix applied,
> udl_drop_usb(udl) is called on error, freeing all URB and DMA
> allocations.
>
> Fixes: 5320918b9a87 ("drm/udl: initial UDL driver (v4)")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> Changes in v2:
> - Split out as patch 2/2 as requested by Jani Nikula.
> - Added testing details in QEMU on drm_dev_register() failure cleanup.
>
> drivers/gpu/drm/udl/udl_drv.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 1922988625eb..239a6983f49f 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -97,8 +97,10 @@ static int udl_usb_probe(struct usb_interface *interface,
> return PTR_ERR(udl);
>
> r = drm_dev_register(&udl->drm, 0);
> - if (r)
> + if (r) {
> + udl_drop_usb(udl);
> return r;
> + }
>
> DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
>
--
--
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)
In udl_usb_probe(), udl_driver_create() calls udl_init(udl), which
allocates USB URBs and DMA buffers via udl_alloc_urb_list() that are
normally freed by udl_drop_usb() in udl_usb_disconnect(). If
drm_dev_register() fails in udl_usb_probe(), udl_usb_probe() returns
without calling udl_drop_usb(udl), leaking the allocated URB list and DMA
buffers because udl_usb_disconnect() is not called when probe fails.
Call udl_drop_usb(udl) when drm_dev_register() fails in udl_usb_probe().
Tested in QEMU against Linux 7.3.0-rc3 using dummy_hcd by simulating
a registration error in drm_dev_register(): on the unfixed kernel,
udl_usb_probe() returned an error without calling udl_drop_usb(udl),
leaking 8 allocated URBs and DMA buffers; whereas with this fix applied,
udl_drop_usb(udl) is called on error, freeing all URB and DMA
allocations.
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>
---
Changes in v3:
- Add Reviewed-by tag from Thomas Zimmermann.
Changes in v2:
- Split out as patch 2/2 as requested by Jani Nikula.
- Added testing details in QEMU on drm_dev_register() failure cleanup.
drivers/gpu/drm/udl/udl_drv.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 1922988625eb..239a6983f49f 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -97,8 +97,10 @@ static int udl_usb_probe(struct usb_interface *interface,
return PTR_ERR(udl);
r = drm_dev_register(&udl->drm, 0);
- if (r)
+ if (r) {
+ udl_drop_usb(udl);
return r;
+ }
DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
On Sat, 19 Sep 2026, Hui Peng <benquike@gmail.com> wrote:
> In drivers/gpu/drm/udl/ (udl_drv.c, udl_edid.c), check that
> usb_control_msg() returns the full 2-byte response in
> udl_read_edid_block() before copying read_buff[1], and clean up
> allocated URBs if udl_Driver initialization fails.
One change per patch please.
>
> Fixes: 5320918b9a87 ("drm/udl: initial UDL driver (v4)")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 1922988625eb..239a6983f49f 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -97,8 +97,10 @@ static int udl_usb_probe(struct usb_interface *interface,
> return PTR_ERR(udl);
>
> r = drm_dev_register(&udl->drm, 0);
> - if (r)
> + if (r) {
> + udl_drop_usb(udl);
> return r;
> + }
>
> DRM_INFO("Initialized udl on minor %d\n", udl->drm.primary->index);
>
> diff --git a/drivers/gpu/drm/udl/udl_edid.c b/drivers/gpu/drm/udl/udl_edid.c
> index af4cff2a7c51..b7c462436573 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;
--
Jani Nikula, Intel
© 2016 - 2026 Red Hat, Inc.