lkml.org 
[lkml]   [2020]   [Aug]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] cdc-acm: rework notification_buffer resizing
From
Date
Am Samstag, den 01.08.2020, 08:21 -0700 schrieb trix@redhat.com:
> From: Tom Rix <trix@redhat.com>
>
> 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: ea2583529cd1 ("cdc-acm: reassemble fragmented notifications")
>
> Signed-off-by: Tom Rix <trix@redhat.com>
Acked-by: Oliver Neukum <oneukum@suse.com>

\
 
 \ /
  Last update: 2020-08-04 11:17    [W:0.040 / U:0.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site