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. Bart
