Re: [PATCH] usb: gadget: f_ecm: serialize notification state with a spinlock

[sashiko-bot@kernel.org]

> ecm_do_notify() guards re-entry only with an atomic_read() of
> notify_count, then fills ecm->notify_req, does atomic_inc() and
> usb_ep_queue().  This check-fill-inc-queue sequence is not atomic and
> runs from three contexts that share no lock: ecm_notify_complete(),
> ecm_open()/ecm_close() (netdev up/down via u_ether) and ecm_set_alt().
> 
> When the completion handler decrements notify_count to 0 and re-enters
> ecm_do_notify(), a concurrent ecm_close()/ecm_set_alt() can observe
> notify_count == 0 in the window between the atomic_dec() and the next
> atomic_inc(), and queue the same ecm->notify_req a second time.  The
> request is then added to the endpoint queue twice (list_add corruption),
> and notify_state is updated concurrently.
> 
> struct f_ecm has carried a FIXME about this missing locking.  The sibling
> f_ncm already serializes the identical state machine with a per-instance
> spinlock.  Do the same in f_ecm: take ecm->lock across the is_open update
> and ecm_notify() in open/close/set_alt and across the state update in
> ecm_notify_complete(), dropping it only around usb_ep_queue().
> 
> Signed-off-by: Yu Zhang <yuz08559@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026060031.60582-1-yuz08559@gmail.com?part=1