On Thu, Aug 29, 2024 at 2:24 PM Vadim Fedorenko <vadim.fedorenko@xxxxxxxxx> wrote: > > On 29/08/2024 22:08, Jakub Kicinski wrote: > > On Thu, 29 Aug 2024 06:01:16 +0000 Mina Almasry wrote: > >> + err = genlmsg_reply(rsp, info); > >> + if (err) > >> + goto err_unbind; > >> + > >> return 0; > >> + > >> +err_unbind: > > > > rtnl_lock() > > There are 2 places with goto err_unbind, and one is under the lock, > additional label (or rearrange of the last check) is needed.. > Thank you, I think the right fix here is to reacquire rtnl_lock before the `goto err_unbind;`, since err_unbind expects rtnl to be locked at this point. This could introduce a weird edge case where we drop rtnl_lock, then find out genlmsg_reply failed, then reacquire rtnl_lock to do the cleanup. I can't think of anything that would horribly break if we do that, but I may be missing something. In theory we could race with a dmabuf unbind call happening in parallel. If we can't reacquire rtnl_lock to do the cleanup, I think I need to revert back to doing genlmsg_reply inside of rtnl_lock, and dropping the lock before we return from the function. -- Thanks, Mina
