Skip to content

Commit f4b9d8a

Browse files
Tom Rixgregkh
authored andcommitted
USB: cdc-acm: rework notification_buffer resizing
Clang static analysis reports this error cdc-acm.c:409:3: warning: Use of memory after it is freed acm_process_notification(acm, (unsigned char *)dr); There are three problems, the first one is that dr is not reset The variable dr is set with if (acm->nb_index) dr = (struct usb_cdc_notification *)acm->notification_buffer; But if the notification_buffer is too small it is resized with if (acm->nb_size) { kfree(acm->notification_buffer); acm->nb_size = 0; } alloc_size = roundup_pow_of_two(expected_size); /* * kmalloc ensures a valid notification_buffer after a * use of kfree in case the previous allocation was too * small. Final freeing is done on disconnect. */ acm->notification_buffer = kmalloc(alloc_size, GFP_ATOMIC); dr should point to the new acm->notification_buffer. The second problem is any data in the notification_buffer is lost when the pointer is freed. In the normal case, the current data is accumulated in the notification_buffer here. memcpy(&acm->notification_buffer[acm->nb_index], urb->transfer_buffer, copy_size); When a resize happens, anything before notification_buffer[acm->nb_index] is garbage. The third problem is the acm->nb_index is not reset on a resizing buffer error. So switch resizing to using krealloc and reassign dr and reset nb_index. Fixes: ea25835 ("cdc-acm: reassemble fragmented notifications") Signed-off-by: Tom Rix <[email protected]> Cc: stable <[email protected]> Acked-by: Oliver Neukum <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 5967116 commit f4b9d8a

File tree

1 file changed

+10
-12
lines changed

1 file changed

+10
-12
lines changed

drivers/usb/class/cdc-acm.c

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -378,21 +378,19 @@ static void acm_ctrl_irq(struct urb *urb)
378378
if (current_size < expected_size) {
379379
/* notification is transmitted fragmented, reassemble */
380380
if (acm->nb_size < expected_size) {
381-
if (acm->nb_size) {
382-
kfree(acm->notification_buffer);
383-
acm->nb_size = 0;
384-
}
381+
u8 *new_buffer;
385382
alloc_size = roundup_pow_of_two(expected_size);
386-
/*
387-
* kmalloc ensures a valid notification_buffer after a
388-
* use of kfree in case the previous allocation was too
389-
* small. Final freeing is done on disconnect.
390-
*/
391-
acm->notification_buffer =
392-
kmalloc(alloc_size, GFP_ATOMIC);
393-
if (!acm->notification_buffer)
383+
/* Final freeing is done on disconnect. */
384+
new_buffer = krealloc(acm->notification_buffer,
385+
alloc_size, GFP_ATOMIC);
386+
if (!new_buffer) {
387+
acm->nb_index = 0;
394388
goto exit;
389+
}
390+
391+
acm->notification_buffer = new_buffer;
395392
acm->nb_size = alloc_size;
393+
dr = (struct usb_cdc_notification *)acm->notification_buffer;
396394
}
397395

398396
copy_size = min(current_size,

0 commit comments

Comments
 (0)