• Benjamin Herrenschmidt's avatar
    usb: gadget: mass_storage: Fix races between fsg_disable and fsg_set_alt · 4a56a478
    Benjamin Herrenschmidt authored
    If fsg_disable() and fsg_set_alt() are called too closely to each
    other (for example due to a quick reset/reconnect), what can happen
    is that fsg_set_alt sets common->new_fsg from an interrupt while
    handle_exception is trying to process the config change caused by
    fsg_disable():
    
    	fsg_disable()
    	...
    	handle_exception()
    		sets state back to FSG_STATE_NORMAL
    		hasn't yet called do_set_interface()
    		or is inside it.
    
     ---> interrupt
    	fsg_set_alt
    		sets common->new_fsg
    		queues a new FSG_STATE_CONFIG_CHANGE
     <---
    
    Now, the first handle_exception can "see" the updated
    new_fsg, treats it as if it was a fsg_set_alt() response,
    call usb_composite_setup_continue() etc...
    
    But then, the thread sees the second FSG_STATE_CONFIG_CHANGE,
    and goes back down the same path, wipes and reattaches a now
    active fsg, and .. calls usb_composite_setup_continue() which
    at this point is wrong.
    
    Not only we get a backtrace, but I suspect the second set_interface
    wrecks some state causing the host to get upset in my case.
    
    This fixes it by replacing "new_fsg" by a "state argument" (same
    principle) which is set in the same lock section as the state
    update, and retrieved similarly.
    
    That way, there is never any discrepancy between the dequeued
    state and the observed value of it. We keep the ability to have
    the latest reconfig operation take precedence, but we guarantee
    that once "dequeued" the argument (new_fsg) will not be clobbered
    by any new event.
    Signed-off-by: default avatarBenjamin Herrenschmidt <benh@kernel.crashing.org>
    Acked-by: default avatarAlan Stern <stern@rowland.harvard.edu>
    Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
    4a56a478
f_mass_storage.c 92.9 KB