• Tom Rix's avatar
    USB: cdc-acm: rework notification_buffer resizing · f4b9d8a5
    Tom Rix authored
    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: ea258352 ("cdc-acm: reassemble fragmented notifications")
    Signed-off-by: default avatarTom Rix <trix@redhat.com>
    Cc: stable <stable@vger.kernel.org>
    Acked-by: default avatarOliver Neukum <oneukum@suse.com>
    Link: https://lore.kernel.org/r/20200801152154.20683-1-trix@redhat.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    f4b9d8a5
cdc-acm.c 53.9 KB