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

[Edward Cree]

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@xxxxxxxxxxxxxx/T/#u