On Wed, Mar 11, 2026 at 01:39:50AM -0700, Bartosz Golaszewski wrote: > On Wed, 11 Mar 2026 03:11:42 +0100, Dmitry Baryshkov > <[email protected]> said: > > On Tue, Mar 10, 2026 at 06:50:30AM -0700, Bartosz Golaszewski wrote: > >> > >> Ideally things like this would be passed to the rproc core in some kind of > >> a > >> config structure and only set when registration succeeds. This looks to me > >> like papering over the real issue and I think it's still racy as there's no > >> true synchronization. > >> > >> Wouldn't it be better to take rproc->lock for the entire duration of > >> rproc_add()? It's already initialized in rproc_alloc(). > > > > It would still be racy as rproc_trigger_recovery() is called outside of > > the lock. Instead the error cleanup path (and BTW, rproc_del() path too) > > must explicitly call cancel_work_sync() on the crash_handler work (and > > any other work items that can be scheduled). > > > > This looks weird TBH. For example: rproc_crash_handler_work() takes the lock, > but releases it right before calling inspecting rproc->recovery_disabled and > calling rproc_trigger_recovery(). It looks wrong, I think it should keep the > lock and rptoc_trigger_recovery() should enforce it being taken before the > call.
Yes. Nevertheless the driver should cancel the work too. -- With best wishes Dmitry
