Re: [PATCH] genirq: fix reference leaks on irq affinity notifiers

[Edward Cree <ecree@solarflare.com>]

On 15/03/2020 20:29, Ben Hutchings wrote:
> ...since the pending work item holds a reference to the notification
> state, it's still not clear to me why or whether "genirq: Prevent use-
> after-free and work list corruption" was needed.
Yeah, I think that commit was bogus.  The email thread[1] doesn't
 exactly inspire confidence either.  I think the submitter just didn't
 realise that there was a ref corresponding to the work; AFAICT there's
 no way the alleged "work list corruption" could happen.

> If it's reasonable to cancel_work_sync() when removing a notifier, I
> think we can remove the kref and call the release function directly.
I'd prefer to stick to the smaller fix for -rc and stable.  But if you
 want to remove the kref for -next, I'd be happy to Ack that patch.


Btw, we (sfc linux team) think there's still a use-after-free issue in
 the cpu_rmap lib, as follows:
1) irq_cpu_rmap_add creates a glue and notifier, adds glue to rmap->obj
2) someone else does irq_set_affinity_notifier.
   This causes cpu_rmap's notifier (old_notify) to get released, and so
   irq_cpu_rmap_release kfrees glue.  But it's still in rmap->obj
3) free_irq_cpu_rmap loops over obj, finds the glue, tries to clear its
   notifier.
Now one could say that this UAF is academic, since having two bits of
 code trying to register notifiers for the same IRQ is broken anyway
 (in this case, the rmap would stop getting updated, because the
 "someone else" stole the notifier).
But I thought I'd bring it up in case it's halfway relevant.

-ed

[1] https://lore.kernel.org/lkml/1553119211-29761-1-git-send-email-psodagud@codeaurora.org/T/#u