* [PATCH] USB: serial: Fix possible memleak in keyspan_port_probe()
@ 2021-10-14 13:20 Wang Hai
2021-10-14 14:11 ` Johan Hovold
0 siblings, 1 reply; 3+ messages in thread
From: Wang Hai @ 2021-10-14 13:20 UTC (permalink / raw)
To: johan, gregkh; +Cc: linux-usb, linux-kernel
I got memory leak as follows when doing fault injection test:
unreferenced object 0xffff888258228440 (size 64):
comm "kworker/7:2", pid 2005, jiffies 4294989509 (age 824.540s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff8167939c>] slab_post_alloc_hook+0x9c/0x490
[<ffffffff8167f627>] kmem_cache_alloc_trace+0x1f7/0x470
[<ffffffffa02ac0e4>] keyspan_port_probe+0xa4/0x5d0 [keyspan]
[<ffffffffa0294c07>] usb_serial_device_probe+0x97/0x1d0 [usbserial]
[<ffffffff82b50ca7>] really_probe+0x167/0x460
[<ffffffff82b51099>] __driver_probe_device+0xf9/0x180
[<ffffffff82b51173>] driver_probe_device+0x53/0x130
[<ffffffff82b516f5>] __device_attach_driver+0x105/0x130
[<ffffffff82b4cfe9>] bus_for_each_drv+0x129/0x190
[<ffffffff82b50a69>] __device_attach+0x1c9/0x270
[<ffffffff82b518d0>] device_initial_probe+0x20/0x30
[<ffffffff82b4f062>] bus_probe_device+0x142/0x160
[<ffffffff82b4a4e9>] device_add+0x829/0x1300
[<ffffffffa0295fda>] usb_serial_probe.cold+0xc9b/0x14ac [usbserial]
[<ffffffffa02266aa>] usb_probe_interface+0x1aa/0x3c0 [usbcore]
[<ffffffff82b50ca7>] really_probe+0x167/0x460
If it fails to allocate memory for an out_buffer[i] or in_buffer[i],
the previously allocated memory for out_buffer or in_buffer needs to
be freed on the error handling path, otherwise a memory leak will result.
Fixes: bad41a5bf177 ("USB: keyspan: fix port DMA-buffer allocations")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Wang Hai <wanghai38@huawei.com>
---
drivers/usb/serial/keyspan.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
index 87b89c99d517..ba27a9f0275b 100644
--- a/drivers/usb/serial/keyspan.c
+++ b/drivers/usb/serial/keyspan.c
@@ -2901,7 +2901,7 @@ static int keyspan_port_probe(struct usb_serial_port *port)
p_priv->inack_buffer = kzalloc(INACK_BUFLEN, GFP_KERNEL);
if (!p_priv->inack_buffer)
- goto err_inack_buffer;
+ goto err_out_buffer;
p_priv->outcont_buffer = kzalloc(OUTCONT_BUFLEN, GFP_KERNEL);
if (!p_priv->outcont_buffer)
@@ -2953,13 +2953,12 @@ static int keyspan_port_probe(struct usb_serial_port *port)
err_outcont_buffer:
kfree(p_priv->inack_buffer);
-err_inack_buffer:
+err_out_buffer:
for (i = 0; i < ARRAY_SIZE(p_priv->out_buffer); ++i)
kfree(p_priv->out_buffer[i]);
-err_out_buffer:
+err_in_buffer:
for (i = 0; i < ARRAY_SIZE(p_priv->in_buffer); ++i)
kfree(p_priv->in_buffer[i]);
-err_in_buffer:
kfree(p_priv);
return -ENOMEM;
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] USB: serial: Fix possible memleak in keyspan_port_probe()
2021-10-14 13:20 [PATCH] USB: serial: Fix possible memleak in keyspan_port_probe() Wang Hai
@ 2021-10-14 14:11 ` Johan Hovold
2021-10-15 7:12 ` wanghai (M)
0 siblings, 1 reply; 3+ messages in thread
From: Johan Hovold @ 2021-10-14 14:11 UTC (permalink / raw)
To: Wang Hai; +Cc: gregkh, linux-usb, linux-kernel
On Thu, Oct 14, 2021 at 09:20:33PM +0800, Wang Hai wrote:
> I got memory leak as follows when doing fault injection test:
>
> unreferenced object 0xffff888258228440 (size 64):
> comm "kworker/7:2", pid 2005, jiffies 4294989509 (age 824.540s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff8167939c>] slab_post_alloc_hook+0x9c/0x490
> [<ffffffff8167f627>] kmem_cache_alloc_trace+0x1f7/0x470
> [<ffffffffa02ac0e4>] keyspan_port_probe+0xa4/0x5d0 [keyspan]
> [<ffffffffa0294c07>] usb_serial_device_probe+0x97/0x1d0 [usbserial]
> [<ffffffff82b50ca7>] really_probe+0x167/0x460
> [<ffffffff82b51099>] __driver_probe_device+0xf9/0x180
> [<ffffffff82b51173>] driver_probe_device+0x53/0x130
> [<ffffffff82b516f5>] __device_attach_driver+0x105/0x130
> [<ffffffff82b4cfe9>] bus_for_each_drv+0x129/0x190
> [<ffffffff82b50a69>] __device_attach+0x1c9/0x270
> [<ffffffff82b518d0>] device_initial_probe+0x20/0x30
> [<ffffffff82b4f062>] bus_probe_device+0x142/0x160
> [<ffffffff82b4a4e9>] device_add+0x829/0x1300
> [<ffffffffa0295fda>] usb_serial_probe.cold+0xc9b/0x14ac [usbserial]
> [<ffffffffa02266aa>] usb_probe_interface+0x1aa/0x3c0 [usbcore]
> [<ffffffff82b50ca7>] really_probe+0x167/0x460
>
> If it fails to allocate memory for an out_buffer[i] or in_buffer[i],
> the previously allocated memory for out_buffer or in_buffer needs to
> be freed on the error handling path, otherwise a memory leak will result.
>
> Fixes: bad41a5bf177 ("USB: keyspan: fix port DMA-buffer allocations")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Wang Hai <wanghai38@huawei.com>
> ---
> drivers/usb/serial/keyspan.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
> index 87b89c99d517..ba27a9f0275b 100644
> --- a/drivers/usb/serial/keyspan.c
> +++ b/drivers/usb/serial/keyspan.c
> @@ -2901,7 +2901,7 @@ static int keyspan_port_probe(struct usb_serial_port *port)
>
> p_priv->inack_buffer = kzalloc(INACK_BUFLEN, GFP_KERNEL);
> if (!p_priv->inack_buffer)
> - goto err_inack_buffer;
> + goto err_out_buffer;
>
> p_priv->outcont_buffer = kzalloc(OUTCONT_BUFLEN, GFP_KERNEL);
> if (!p_priv->outcont_buffer)
> @@ -2953,13 +2953,12 @@ static int keyspan_port_probe(struct usb_serial_port *port)
>
> err_outcont_buffer:
> kfree(p_priv->inack_buffer);
> -err_inack_buffer:
> +err_out_buffer:
> for (i = 0; i < ARRAY_SIZE(p_priv->out_buffer); ++i)
> kfree(p_priv->out_buffer[i]);
> -err_out_buffer:
> +err_in_buffer:
> for (i = 0; i < ARRAY_SIZE(p_priv->in_buffer); ++i)
> kfree(p_priv->in_buffer[i]);
> -err_in_buffer:
> kfree(p_priv);
>
> return -ENOMEM;
Good catch. Fortunately these small allocations would currently never
fail, but we should fix it up nonetheless.
The fix looks correct, but you're now mixing two styles of error labels
(i.e. naming them after where you jump from and after what they do,
respectively).
Since you're touching all but one label, could you rename also the last
one after what is done and include a "free_" infix in the label names
(e.g. err_free_in_buffer, etc)?
Johan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] USB: serial: Fix possible memleak in keyspan_port_probe()
2021-10-14 14:11 ` Johan Hovold
@ 2021-10-15 7:12 ` wanghai (M)
0 siblings, 0 replies; 3+ messages in thread
From: wanghai (M) @ 2021-10-15 7:12 UTC (permalink / raw)
To: Johan Hovold; +Cc: gregkh, linux-usb, linux-kernel
在 2021/10/14 22:11, Johan Hovold 写道:
> On Thu, Oct 14, 2021 at 09:20:33PM +0800, Wang Hai wrote:
>> I got memory leak as follows when doing fault injection test:
>>
>> unreferenced object 0xffff888258228440 (size 64):
>> comm "kworker/7:2", pid 2005, jiffies 4294989509 (age 824.540s)
>> hex dump (first 32 bytes):
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>> backtrace:
>> [<ffffffff8167939c>] slab_post_alloc_hook+0x9c/0x490
>> [<ffffffff8167f627>] kmem_cache_alloc_trace+0x1f7/0x470
>> [<ffffffffa02ac0e4>] keyspan_port_probe+0xa4/0x5d0 [keyspan]
>> [<ffffffffa0294c07>] usb_serial_device_probe+0x97/0x1d0 [usbserial]
>> [<ffffffff82b50ca7>] really_probe+0x167/0x460
>> [<ffffffff82b51099>] __driver_probe_device+0xf9/0x180
>> [<ffffffff82b51173>] driver_probe_device+0x53/0x130
>> [<ffffffff82b516f5>] __device_attach_driver+0x105/0x130
>> [<ffffffff82b4cfe9>] bus_for_each_drv+0x129/0x190
>> [<ffffffff82b50a69>] __device_attach+0x1c9/0x270
>> [<ffffffff82b518d0>] device_initial_probe+0x20/0x30
>> [<ffffffff82b4f062>] bus_probe_device+0x142/0x160
>> [<ffffffff82b4a4e9>] device_add+0x829/0x1300
>> [<ffffffffa0295fda>] usb_serial_probe.cold+0xc9b/0x14ac [usbserial]
>> [<ffffffffa02266aa>] usb_probe_interface+0x1aa/0x3c0 [usbcore]
>> [<ffffffff82b50ca7>] really_probe+0x167/0x460
>>
>> If it fails to allocate memory for an out_buffer[i] or in_buffer[i],
>> the previously allocated memory for out_buffer or in_buffer needs to
>> be freed on the error handling path, otherwise a memory leak will result.
>>
>> Fixes: bad41a5bf177 ("USB: keyspan: fix port DMA-buffer allocations")
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Wang Hai <wanghai38@huawei.com>
>> ---
>> drivers/usb/serial/keyspan.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/serial/keyspan.c b/drivers/usb/serial/keyspan.c
>> index 87b89c99d517..ba27a9f0275b 100644
>> --- a/drivers/usb/serial/keyspan.c
>> +++ b/drivers/usb/serial/keyspan.c
>> @@ -2901,7 +2901,7 @@ static int keyspan_port_probe(struct usb_serial_port *port)
>>
>> p_priv->inack_buffer = kzalloc(INACK_BUFLEN, GFP_KERNEL);
>> if (!p_priv->inack_buffer)
>> - goto err_inack_buffer;
>> + goto err_out_buffer;
>>
>> p_priv->outcont_buffer = kzalloc(OUTCONT_BUFLEN, GFP_KERNEL);
>> if (!p_priv->outcont_buffer)
>> @@ -2953,13 +2953,12 @@ static int keyspan_port_probe(struct usb_serial_port *port)
>>
>> err_outcont_buffer:
>> kfree(p_priv->inack_buffer);
>> -err_inack_buffer:
>> +err_out_buffer:
>> for (i = 0; i < ARRAY_SIZE(p_priv->out_buffer); ++i)
>> kfree(p_priv->out_buffer[i]);
>> -err_out_buffer:
>> +err_in_buffer:
>> for (i = 0; i < ARRAY_SIZE(p_priv->in_buffer); ++i)
>> kfree(p_priv->in_buffer[i]);
>> -err_in_buffer:
>> kfree(p_priv);
>>
>> return -ENOMEM;
> Good catch. Fortunately these small allocations would currently never
> fail, but we should fix it up nonetheless.
>
> The fix looks correct, but you're now mixing two styles of error labels
> (i.e. naming them after where you jump from and after what they do,
> respectively).
>
> Since you're touching all but one label, could you rename also the last
> one after what is done and include a "free_" infix in the label names
> (e.g. err_free_in_buffer, etc)?
Okay, thank you for your suggestion, I will send the v2 patch.
>
> Johan
> .
>
--
Wang Hai
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-15 7:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-14 13:20 [PATCH] USB: serial: Fix possible memleak in keyspan_port_probe() Wang Hai
2021-10-14 14:11 ` Johan Hovold
2021-10-15 7:12 ` wanghai (M)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).