Re: [PATCH] usb: typec: ucsi: fix teardown races with late notifications

[Huang Wei <huangwei@kylinos.cn>]

Hi Iván,

I've been poking at the ucsi teardown path lately so I took a closer
look at this. The race is real, and moving acpi_remove_notify_handler()
ahead of ucsi_unregister()/ucsi_destroy() is clearly the right call for
the ACPI path.

One thing I couldn't convince myself of: does acpi_remove_notify_handler()
wait for a handler that's already running on another CPU? From what I
can tell ACPICA removes the handler node under the device lock but
doesn't flush an in-flight dispatch, so a notify that's already entered
ucsi_acpi_notify() could still dereference ua->ucsi after the handler
is gone and ucsi_destroy() has freed it. The reorder handles new
notifies fine, I just wasn't sure it handles the one that's already
mid-flight. Did you look at that?

On the ucsi.c changes: setting ntfy to 0 before NULLing the connector
array looks right to me, ucsi_connector_change() bails on ntfy before
it touches the connector. But ucsi_notify_common() also does
complete(&ucsi->complete) and the backend's read_cci(), neither of
which is gated by ntfy. So for the non-ACPI backends (glink, ccg)
that can't be fixed by reordering the ACPI handler -- is the ucsi.c
part actually enough on its own, or do they need their own teardown
ordering? Put differently, is the ACPI reorder the only real fix and
the rest belt-and-suspenders?

Only build-tested too -- would help to know how the race was found
and what makes the reorder sufficient on its own.

Not objecting, I'd just like to understand the in-flight case.

Thanks,
Huang Wei