[PATCH] usb: image: mdc800: change kmalloc() to kzalloc()

[PATCH] usb: image: mdc800: change kmalloc() to kzalloc()

[Griffin Kroah-Hartman]

Change the kmalloc() calls in usb_mdc800_init() for irq_urb_buffer and
download_urb_buffer to kzalloc(), avoiding potential stack leaks if a
shorter message is received in mdc800_usb_irq() and
mdc800_usb_download_notify()

Assisted-by: gkh_clanker_t1000
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
 drivers/usb/image/mdc800.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/image/mdc800.c b/drivers/usb/image/mdc800.c
index ca287b770e8c..f7caa1c5cbb7 100644
--- a/drivers/usb/image/mdc800.c
+++ b/drivers/usb/image/mdc800.c
@@ -1000,13 +1000,13 @@ static int __init usb_mdc800_init (void)
 	mdc800->downloaded = 0;
 	mdc800->written = 0;
 
-	mdc800->irq_urb_buffer=kmalloc (8, GFP_KERNEL);
+	mdc800->irq_urb_buffer=kzalloc (8, GFP_KERNEL);
 	if (!mdc800->irq_urb_buffer)
 		goto cleanup_on_fail;
 	mdc800->write_urb_buffer=kmalloc (8, GFP_KERNEL);
 	if (!mdc800->write_urb_buffer)
 		goto cleanup_on_fail;
-	mdc800->download_urb_buffer=kmalloc (64, GFP_KERNEL);
+	mdc800->download_urb_buffer=kzalloc (64, GFP_KERNEL);
 	if (!mdc800->download_urb_buffer)
 		goto cleanup_on_fail;
 

---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260817-usb_misc_random-41741bfe9b52

Best regards,
--  
Griffin Kroah-Hartman <griffin@kroah.com>

Re: [PATCH] usb: image: mdc800: change kmalloc() to kzalloc()

[Oliver Neukum]

On 19.08.26 12:04, Griffin Kroah-Hartman wrote:
> Change the kmalloc() calls in usb_mdc800_init() for irq_urb_buffer and
> download_urb_buffer to kzalloc(), avoiding potential stack leaks if a
> shorter message is received in mdc800_usb_irq() and
> mdc800_usb_download_notify()

Hi,

thanks for the patch, but is this the right approach?
If we get too short a reply we shouldn't process it
at all.

	Regards
		Oliver

Re: [PATCH] usb: image: mdc800: change kmalloc() to kzalloc()

[Greg Kroah-Hartman]

On Tue, Sep 01, 2026 at 02:17:54PM +0200, Oliver Neukum wrote:
> 
> 
> On 19.08.26 12:04, Griffin Kroah-Hartman wrote:
> > Change the kmalloc() calls in usb_mdc800_init() for irq_urb_buffer and
> > download_urb_buffer to kzalloc(), avoiding potential stack leaks if a
> > shorter message is received in mdc800_usb_irq() and
> > mdc800_usb_download_notify()
> 
> Hi,
> 
> thanks for the patch, but is this the right approach?
> If we get too short a reply we shouldn't process it
> at all.

The last time we tried to do that, we broke a bunch of CAN drivers
because they ended up sending "short" messages that no one noticed :(

With old hardware like this, let's just stick to this "obviously
correct" patch, and if someone with the hardware ever shows up, we can
then test short message information like this.  As-is, this prevents a
simple "leak kernel memory to userspace with a hand-crafted-device" bug.

thanks,

greg k-h

Re: [PATCH] usb: image: mdc800: change kmalloc() to kzalloc()

[Oliver Neukum]

On 01.09.26 16:30, Greg Kroah-Hartman wrote:

> With old hardware like this, let's just stick to this "obviously
> correct" patch, and if someone with the hardware ever shows up, we can
> then test short message information like this.  As-is, this prevents a
> simple "leak kernel memory to userspace with a hand-crafted-device" bug.

You are not wrong, but if I put on my security person hat I cannot
ignore that we are still leaking information among users. That is
the obvious fixes would be memsets in open(), not in init()

	Regards
		Oliver