USB: cdc-acm: rework notification_buffer resizing
commitf4b9d8a582
upstream. 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:ea2583529c
("cdc-acm: reassemble fragmented notifications") Signed-off-by: Tom Rix <trix@redhat.com> Cc: stable <stable@vger.kernel.org> Acked-by: Oliver Neukum <oneukum@suse.com> Link: https://lore.kernel.org/r/20200801152154.20683-1-trix@redhat.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
e2e02f2604
commit
2add73c8c0
|
@ -378,21 +378,19 @@ static void acm_ctrl_irq(struct urb *urb)
|
|||
if (current_size < expected_size) {
|
||||
/* notification is transmitted fragmented, reassemble */
|
||||
if (acm->nb_size < expected_size) {
|
||||
if (acm->nb_size) {
|
||||
kfree(acm->notification_buffer);
|
||||
acm->nb_size = 0;
|
||||
}
|
||||
u8 *new_buffer;
|
||||
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);
|
||||
if (!acm->notification_buffer)
|
||||
/* Final freeing is done on disconnect. */
|
||||
new_buffer = krealloc(acm->notification_buffer,
|
||||
alloc_size, GFP_ATOMIC);
|
||||
if (!new_buffer) {
|
||||
acm->nb_index = 0;
|
||||
goto exit;
|
||||
}
|
||||
|
||||
acm->notification_buffer = new_buffer;
|
||||
acm->nb_size = alloc_size;
|
||||
dr = (struct usb_cdc_notification *)acm->notification_buffer;
|
||||
}
|
||||
|
||||
copy_size = min(current_size,
|
||||
|
|
Loading…
Reference in New Issue